Show in-product help when WebUI tab strip is first shown

Before, an IPH bubble was shown when a second tab was opened. With
this CL it is shown as soon as possible after the WebUI tab strip is
initialized.

Bug: None
Change-Id: I2f3e7a7dd04229953af5eefe99e00dbcbbfaa8da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418566
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810097}

chrome/browser/ui/views/frame/browser_view.cc

@@ -37,6 +37,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/browser_extension_window_controller.h"
 #include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/feature_engagement/tracker_factory.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/native_window_notification_source.h"
 #include "chrome/browser/platform_util.h"
@@ -144,6 +145,8 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "components/autofill/core/common/autofill_payments_features.h"
+#include "components/feature_engagement/public/feature_constants.h"
+#include "components/feature_engagement/public/tracker.h"
 #include "components/javascript_dialogs/app_modal_dialog_controller.h"
 #include "components/javascript_dialogs/app_modal_dialog_queue.h"
 #include "components/javascript_dialogs/app_modal_dialog_view.h"
@@ -1580,6 +1583,25 @@ void BrowserView::TryNotifyWindowBoundsChanged(const gfx::Rect& widget_bounds) {
   browser()->extension_window_controller()->NotifyWindowBoundsChanged();
 }
 
+void BrowserView::TouchModeChanged() {
+  MaybeInitializeWebUITabStrip();
+  MaybeShowWebUITabStripIPH();
+}
+
+void BrowserView::OnFeatureEngagementTrackerInitialized(bool initialized) {
+  if (!initialized)
+    return;
+  MaybeShowWebUITabStripIPH();
+}
+
+void BrowserView::MaybeShowWebUITabStripIPH() {
+  if (!webui_tab_strip_)
+    return;
+
+  feature_promo_controller_->MaybeShowPromo(
+      feature_engagement::kIPHWebUITabStripFeature);
+}
+
 void BrowserView::DestroyBrowser() {
   // After this returns other parts of Chrome are going to be shutdown. Close
   // the window now so that we are deleted immediately and aren't left holding
@@ -2854,6 +2876,11 @@ void BrowserView::AddedToWidget() {
 
   MaybeInitializeWebUITabStrip();
 
+  feature_promo_controller_->feature_engagement_tracker()
+      ->AddOnInitializedCallback(
+          base::BindOnce(&BrowserView::OnFeatureEngagementTrackerInitialized,
+                         weak_ptr_factory_.GetWeakPtr()));
+
   initialized_ = true;
 }
 

chrome/browser/ui/views/frame/browser_view.h

@@ -755,6 +755,17 @@ class BrowserView : public BrowserWindow,
   // Notifies that window bounds changed to extensions if needed.
   void TryNotifyWindowBoundsChanged(const gfx::Rect& widget_bounds);
 
+  // Called when ui::TouchUiController changes the current touch mode.
+  void TouchModeChanged();
+
+  // Called when the in-product help backend is initialized.
+  void OnFeatureEngagementTrackerInitialized(bool initialized);
+
+  // Attempts to show in-product help for the WebUI tab strip. Should be
+  // called when the IPH backend is initialized or whenever the touch
+  // mode changes.
+  void MaybeShowWebUITabStripIPH();
+
   // The BrowserFrame that hosts this view.
   BrowserFrame* frame_ = nullptr;
 
@@ -905,7 +916,7 @@ class BrowserView : public BrowserWindow,
 
   std::unique_ptr<ui::TouchUiController::Subscription> subscription_ =
       ui::TouchUiController::Get()->RegisterCallback(
-          base::BindRepeating(&BrowserView::MaybeInitializeWebUITabStrip,
+          base::BindRepeating(&BrowserView::TouchModeChanged,
                               base::Unretained(this)));
 
   std::unique_ptr<WebContentsCloseHandler> web_contents_close_handler_;

chrome/browser/ui/views/frame/webui_tab_strip_container_view.cc

@@ -38,8 +38,6 @@
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/frame/top_container_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_view.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_colors.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_controller_views.h"
 #include "chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.h"
@@ -440,75 +438,6 @@ class WebUITabStripContainerView::DragToOpenHandler : public ui::EventHandler {
   bool drag_in_progress_ = false;
 };
 
-class WebUITabStripContainerView::IPHController : public TabStripModelObserver {
- public:
-  explicit IPHController(Browser* browser,
-                         FeaturePromoControllerViews* promo_controller)
-      : browser_(browser),
-        promo_controller_(promo_controller),
-        iph_tracker_(feature_engagement::TrackerFactory::GetForBrowserContext(
-            browser_->profile())) {
-    browser_->tab_strip_model()->AddObserver(this);
-  }
-
-  ~IPHController() override {
-    browser_->tab_strip_model()->RemoveObserver(this);
-  }
-
-  void SetAnchorView(views::View* anchor_view) {
-    DCHECK(!anchor_.view());
-    anchor_.SetView(anchor_view);
-  }
-
-  void NotifyOpened() {
-    iph_tracker_->NotifyEvent(feature_engagement::events::kWebUITabStripOpened);
-  }
-
-  void NotifyClosed() {
-    iph_tracker_->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
-  }
-
-  // Ends the promo if it's showing.
-  void AbortPromo() {
-    if (!promo_controller_->BubbleIsShowing(
-            feature_engagement::kIPHWebUITabStripFeature))
-      return;
-    promo_controller_->CloseBubble(
-        feature_engagement::kIPHWebUITabStripFeature);
-  }
-
-  // TabStripModelObserver:
-  void OnTabStripModelChanged(
-      TabStripModel* tab_strip_model,
-      const TabStripModelChange& change,
-      const TabStripSelectionChange& selection) override {
-    // We want to show the IPH to let the user know where their new tabs
-    // are. So, ignore changes other than insertions.
-    if (change.type() != TabStripModelChange::kInserted)
-      return;
-
-    views::View* const anchor_view = anchor_.view();
-
-    // In the off chance this is called while the browser is being destroyed,
-    // return.
-    if (!anchor_view)
-      return;
-
-    FeaturePromoBubbleParams bubble_params;
-    bubble_params.body_string_specifier = IDS_WEBUI_TAB_STRIP_PROMO;
-    bubble_params.anchor_view = anchor_view;
-    bubble_params.arrow = views::BubbleBorder::TOP_RIGHT;
-    promo_controller_->MaybeShowPromoWithParams(
-        feature_engagement::kIPHWebUITabStripFeature, bubble_params);
-  }
-
- private:
-  Browser* const browser_;
-  FeaturePromoControllerViews* const promo_controller_;
-  feature_engagement::Tracker* const iph_tracker_;
-  views::ViewTracker anchor_;
-};
-
 WebUITabStripContainerView::WebUITabStripContainerView(
     BrowserView* browser_view,
     views::View* tab_contents_container,
@@ -526,10 +455,7 @@ WebUITabStripContainerView::WebUITabStripContainerView(
           tab_contents_container,
           omnibox)),
       drag_to_open_handler_(
-          std::make_unique<DragToOpenHandler>(this, top_container)),
-      iph_controller_(std::make_unique<IPHController>(
-          browser_view->browser(),
-          browser_view->feature_promo_controller())) {
+          std::make_unique<DragToOpenHandler>(this, top_container)) {
   TRACE_EVENT0("ui", "WebUITabStripContainerView.Init");
   DCHECK(UseTouchableTabStrip(browser_view_->browser()));
 
@@ -639,8 +565,6 @@ std::unique_ptr<views::View> WebUITabStripContainerView::CreateTabCounter() {
   tab_counter_ = tab_counter.get();
   view_observer_.Add(tab_counter_);
 
-  iph_controller_->SetAnchorView(tab_counter_);
-
   return tab_counter;
 }
 
@@ -665,7 +589,9 @@ WebUITabStripContainerView::GetAcceleratorProvider() const {
 
 void WebUITabStripContainerView::CloseContainer() {
   SetContainerTargetVisibility(false, WebUITabStripOpenCloseReason::kOther);
-  iph_controller_->NotifyClosed();
+  browser_view_->feature_promo_controller()
+      ->feature_engagement_tracker()
+      ->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
 }
 
 bool WebUITabStripContainerView::CanStartDragToOpen(
@@ -715,7 +641,13 @@ void WebUITabStripContainerView::EndDragToOpen(
 
   if (opening) {
     RecordTabStripUIOpenHistogram(TabStripUIOpenAction::kToolbarDrag);
-    iph_controller_->NotifyOpened();
+    browser_view_->feature_promo_controller()
+        ->feature_engagement_tracker()
+        ->NotifyEvent(feature_engagement::events::kWebUITabStripOpened);
+  } else {
+    browser_view_->feature_promo_controller()
+        ->feature_engagement_tracker()
+        ->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
   }
 
   animation_.Reset(open_proportion);
@@ -752,8 +684,11 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
 
     time_at_open_ = base::TimeTicks::Now();
 
-    // If we're opening, end IPH if it's showing.
-    iph_controller_->AbortPromo();
+    if (browser_view_->feature_promo_controller()->BubbleIsShowing(
+            feature_engagement::kIPHWebUITabStripFeature)) {
+      browser_view_->feature_promo_controller()->CloseBubble(
+          feature_engagement::kIPHWebUITabStripFeature);
+    }
   } else {
     if (time_at_open_) {
       RecordTabStripUIOpenDurationHistogram(base::TimeTicks::Now() -
@@ -783,7 +718,6 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
 void WebUITabStripContainerView::CloseForEventOutsideTabStrip(
     TabStripUICloseAction reason) {
   RecordTabStripUICloseHistogram(reason);
-  iph_controller_->NotifyClosed();
   SetContainerTargetVisibility(false, WebUITabStripOpenCloseReason::kOther);
 }
 
@@ -867,10 +801,14 @@ void WebUITabStripContainerView::ButtonPressed(views::Button* sender,
   const bool new_visibility = !GetVisible();
   if (new_visibility) {
     RecordTabStripUIOpenHistogram(TabStripUIOpenAction::kTapOnTabCounter);
-    iph_controller_->NotifyOpened();
+    browser_view_->feature_promo_controller()
+        ->feature_engagement_tracker()
+        ->NotifyEvent(feature_engagement::events::kWebUITabStripOpened);
   } else {
     RecordTabStripUICloseHistogram(TabStripUICloseAction::kTapOnTabCounter);
-    iph_controller_->NotifyClosed();
+    browser_view_->feature_promo_controller()
+        ->feature_engagement_tracker()
+        ->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
   }
 
   SetContainerTargetVisibility(new_visibility,

chrome/browser/ui/views/frame/webui_tab_strip_container_view.h

@@ -71,13 +71,14 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
   // Control button. Must only be called once.
   std::unique_ptr<views::View> CreateTabCounter();
 
+  views::View* tab_counter() const { return tab_counter_; }
+
   // Clicking the tab counter button opens and closes the container with
   // an animation, so it is unsuitable for an interactive test. This
   // should be called instead. View::SetVisible() isn't sufficient since
   // the container's preferred size will change.
   void SetVisibleForTesting(bool visible);
   views::WebView* web_view_for_testing() const { return web_view_; }
-  views::View* tab_counter_for_testing() const { return tab_counter_; }
 
   // Finish the open or close animation if it's active.
   void FinishAnimationForTesting();
@@ -85,7 +86,6 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
  private:
   class AutoCloser;
   class DragToOpenHandler;
-  class IPHController;
 
   // Called as we are dragged open.
   bool CanStartDragToOpen(WebUITabStripDragDirection direction) const;
@@ -159,7 +159,6 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
 
   std::unique_ptr<AutoCloser> auto_closer_;
   std::unique_ptr<DragToOpenHandler> drag_to_open_handler_;
-  std::unique_ptr<IPHController> iph_controller_;
 
   std::unique_ptr<views::MenuRunner> context_menu_runner_;
   std::unique_ptr<ui::MenuModel> context_menu_model_;

chrome/browser/ui/views/frame/webui_tab_strip_container_view_unittest.cc

@@ -65,10 +65,9 @@ TEST_F(WebUITabStripContainerViewTest, TouchModeTransition) {
 }
 
 TEST_F(WebUITabStripContainerViewTest, ButtonsPresentInToolbar) {
-  ASSERT_NE(nullptr,
-            browser_view()->webui_tab_strip()->tab_counter_for_testing());
+  ASSERT_NE(nullptr, browser_view()->webui_tab_strip()->tab_counter());
   EXPECT_TRUE(browser_view()->toolbar()->Contains(
-      browser_view()->webui_tab_strip()->tab_counter_for_testing()));
+      browser_view()->webui_tab_strip()->tab_counter()));
 }
 
 TEST_F(WebUITabStripContainerViewTest, PreventsInvalidTabDrags) {
@@ -156,62 +155,4 @@ TEST_F(WebUITabStripDevToolsTest, DevToolsWindowHasNoTabStrip) {
   EXPECT_EQ(nullptr, browser_view()->webui_tab_strip());
 }
 
-class WebUITabStripIPHTest : public WebUITabStripContainerViewTest {
- public:
-  void SetUp() override {
-    WebUITabStripContainerViewTest::SetUp();
-
-    mock_tracker_ = static_cast<MockTracker*>(
-        feature_engagement::TrackerFactory::GetForBrowserContext(
-            browser()->profile()));
-  }
-
- protected:
-  using MockTracker =
-      ::testing::NiceMock<feature_engagement::test::MockTracker>;
-
-  TestingProfile::TestingFactories GetTestingFactories() override {
-    auto factories = WebUITabStripContainerViewTest::GetTestingFactories();
-    factories.emplace_back(feature_engagement::TrackerFactory::GetInstance(),
-                           base::Bind(MakeMockTracker));
-    return factories;
-  }
-
-  MockTracker* mock_tracker_;
-
- private:
-  static std::unique_ptr<KeyedService> MakeMockTracker(
-      content::BrowserContext* _context) {
-    using ::testing::_;
-    using ::testing::AnyNumber;
-    using ::testing::Return;
-
-    auto mock_tracker = std::make_unique<MockTracker>();
-
-    // By default, allow calls to ShouldTriggerHelpUI. Other features
-    // may query this. We will set WebUI tab strip-specific expectations
-    // later.
-    EXPECT_CALL(*mock_tracker, ShouldTriggerHelpUI(_))
-        .Times(AnyNumber())
-        .WillRepeatedly(Return(false));
-    return mock_tracker;
-  }
-};
-
-TEST_F(WebUITabStripIPHTest, OpeningNewTabAttemptsIPH) {
-  using ::testing::Eq;
-  using ::testing::Field;
-  using ::testing::Return;
-
-  // Ensure the IPH attempts to show upon opening a new tab.
-  EXPECT_CALL(*mock_tracker_,
-              ShouldTriggerHelpUI(
-                  Field(&base::Feature::name,
-                        Eq(feature_engagement::kIPHWebUITabStripFeature.name))))
-      .Times(1)
-      .WillOnce(Return(false));
-
-  chrome::NewTab(browser());
-}
-
 // TODO(crbug.com/1066624): add coverage of open and close gestures.

chrome/browser/ui/views/in_product_help/feature_promo_controller_views.h

@@ -57,6 +57,9 @@ class FeaturePromoControllerViews : public FeaturePromoController,
   void OnWidgetClosing(views::Widget* widget) override;
   void OnWidgetDestroying(views::Widget* widget) override;
 
+  // Gets the IPH backend. Provided for convenience.
+  feature_engagement::Tracker* feature_engagement_tracker() { return tracker_; }
+
   FeaturePromoBubbleView* promo_bubble_for_testing() { return promo_bubble_; }
   const FeaturePromoBubbleView* promo_bubble_for_testing() const {
     return promo_bubble_;

chrome/browser/ui/views/in_product_help/feature_promo_registry.cc

@@ -11,9 +11,14 @@
 #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_params.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/common/buildflags.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/feature_engagement/public/feature_constants.h"
 
+#if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
+#include "chrome/browser/ui/views/frame/webui_tab_strip_container_view.h"
+#endif  // BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
+
 namespace {
 
 // Functions to get an anchor view for an IPH should go here.
@@ -30,6 +35,17 @@ views::View* GetMediaButton(BrowserView* browser_view) {
   return browser_view->toolbar()->media_button();
 }
 
+#if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
+// kIPHWebUITabStripFeature:
+views::View* GetWebUITabStripAnchorView(BrowserView* browser_view) {
+  WebUITabStripContainerView* const webui_tab_strip =
+      browser_view->webui_tab_strip();
+  if (!webui_tab_strip)
+    return nullptr;
+  return webui_tab_strip->tab_counter();
+}
+#endif  // BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
+
 }  // namespace
 
 FeaturePromoRegistry::FeaturePromoRegistry() {
@@ -108,6 +124,18 @@ void FeaturePromoRegistry::RegisterKnownFeatures() {
     RegisterFeature(feature_engagement::kIPHLiveCaptionFeature, params,
                     base::BindRepeating(GetMediaButton));
   }
+
+#if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
+  {
+    // kIPHWebUITabStripFeature:
+    FeaturePromoBubbleParams params;
+    params.body_string_specifier = IDS_WEBUI_TAB_STRIP_PROMO;
+    params.arrow = views::BubbleBorder::TOP_RIGHT;
+
+    RegisterFeature(feature_engagement::kIPHWebUITabStripFeature, params,
+                    base::BindRepeating(GetWebUITabStripAnchorView));
+  }
+#endif  // BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
 }
 
 FeaturePromoRegistry::FeaturePromoData::FeaturePromoData() = default;