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

This reverts commit 944aea5f.

Reason for revert: I suspect this is causing
WebUITabStripInteractiveTest.CanUseInImmersiveMode to fail

Bug: 1131931

Original change's description:
> 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}

TBR=dfried@chromium.org,collinbaker@chromium.org

Change-Id: I9c3a8fba5c893d28acf392f86dc6e27cf9b4cb15
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428627Reviewed-by: Nina Satragno <nsatragno@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810204}

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

@@ -37,7 +37,6 @@
 #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"
@@ -145,8 +144,6 @@
 #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"
@@ -1583,25 +1580,6 @@ 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
@@ -2876,11 +2854,6 @@ 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,17 +755,6 @@ 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;
 
@@ -916,7 +905,7 @@ class BrowserView : public BrowserWindow,
 
   std::unique_ptr<ui::TouchUiController::Subscription> subscription_ =
       ui::TouchUiController::Get()->RegisterCallback(
-          base::BindRepeating(&BrowserView::TouchModeChanged,
+          base::BindRepeating(&BrowserView::MaybeInitializeWebUITabStrip,
                               base::Unretained(this)));
 
   std::unique_ptr<WebContentsCloseHandler> web_contents_close_handler_;

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

@@ -38,6 +38,8 @@
 #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"
@@ -438,6 +440,75 @@ 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,
@@ -455,7 +526,10 @@ WebUITabStripContainerView::WebUITabStripContainerView(
           tab_contents_container,
           omnibox)),
       drag_to_open_handler_(
-          std::make_unique<DragToOpenHandler>(this, top_container)) {
+          std::make_unique<DragToOpenHandler>(this, top_container)),
+      iph_controller_(std::make_unique<IPHController>(
+          browser_view->browser(),
+          browser_view->feature_promo_controller())) {
   TRACE_EVENT0("ui", "WebUITabStripContainerView.Init");
   DCHECK(UseTouchableTabStrip(browser_view_->browser()));
 
@@ -565,6 +639,8 @@ 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;
 }
 
@@ -589,9 +665,7 @@ WebUITabStripContainerView::GetAcceleratorProvider() const {
 
 void WebUITabStripContainerView::CloseContainer() {
   SetContainerTargetVisibility(false, WebUITabStripOpenCloseReason::kOther);
-  browser_view_->feature_promo_controller()
-      ->feature_engagement_tracker()
-      ->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
+  iph_controller_->NotifyClosed();
 }
 
 bool WebUITabStripContainerView::CanStartDragToOpen(
@@ -641,13 +715,7 @@ void WebUITabStripContainerView::EndDragToOpen(
 
   if (opening) {
     RecordTabStripUIOpenHistogram(TabStripUIOpenAction::kToolbarDrag);
-    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);
+    iph_controller_->NotifyOpened();
   }
 
   animation_.Reset(open_proportion);
@@ -684,11 +752,8 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
 
     time_at_open_ = base::TimeTicks::Now();
 
-    if (browser_view_->feature_promo_controller()->BubbleIsShowing(
-            feature_engagement::kIPHWebUITabStripFeature)) {
-      browser_view_->feature_promo_controller()->CloseBubble(
-          feature_engagement::kIPHWebUITabStripFeature);
-    }
+    // If we're opening, end IPH if it's showing.
+    iph_controller_->AbortPromo();
   } else {
     if (time_at_open_) {
       RecordTabStripUIOpenDurationHistogram(base::TimeTicks::Now() -
@@ -718,6 +783,7 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
 void WebUITabStripContainerView::CloseForEventOutsideTabStrip(
     TabStripUICloseAction reason) {
   RecordTabStripUICloseHistogram(reason);
+  iph_controller_->NotifyClosed();
   SetContainerTargetVisibility(false, WebUITabStripOpenCloseReason::kOther);
 }
 
@@ -801,14 +867,10 @@ void WebUITabStripContainerView::ButtonPressed(views::Button* sender,
   const bool new_visibility = !GetVisible();
   if (new_visibility) {
     RecordTabStripUIOpenHistogram(TabStripUIOpenAction::kTapOnTabCounter);
-    browser_view_->feature_promo_controller()
-        ->feature_engagement_tracker()
-        ->NotifyEvent(feature_engagement::events::kWebUITabStripOpened);
+    iph_controller_->NotifyOpened();
   } else {
     RecordTabStripUICloseHistogram(TabStripUICloseAction::kTapOnTabCounter);
-    browser_view_->feature_promo_controller()
-        ->feature_engagement_tracker()
-        ->NotifyEvent(feature_engagement::events::kWebUITabStripClosed);
+    iph_controller_->NotifyClosed();
   }
 
   SetContainerTargetVisibility(new_visibility,

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

@@ -71,14 +71,13 @@ 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();
@@ -86,6 +85,7 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
  private:
   class AutoCloser;
   class DragToOpenHandler;
+  class IPHController;
 
   // Called as we are dragged open.
   bool CanStartDragToOpen(WebUITabStripDragDirection direction) const;
@@ -159,6 +159,7 @@ 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,9 +65,10 @@ TEST_F(WebUITabStripContainerViewTest, TouchModeTransition) {
 }
 
 TEST_F(WebUITabStripContainerViewTest, ButtonsPresentInToolbar) {
-  ASSERT_NE(nullptr, browser_view()->webui_tab_strip()->tab_counter());
+  ASSERT_NE(nullptr,
+            browser_view()->webui_tab_strip()->tab_counter_for_testing());
   EXPECT_TRUE(browser_view()->toolbar()->Contains(
-      browser_view()->webui_tab_strip()->tab_counter()));
+      browser_view()->webui_tab_strip()->tab_counter_for_testing()));
 }
 
 TEST_F(WebUITabStripContainerViewTest, PreventsInvalidTabDrags) {
@@ -155,4 +156,62 @@ 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,9 +57,6 @@ 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,14 +11,9 @@
 #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.
@@ -35,17 +30,6 @@ 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() {
@@ -124,18 +108,6 @@ 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;