Remove omnibox focus from reopen tab in-product help trigger conditions

The omnibox is always automatically focused after a blank new tab is
opened. Hence, we don't have to consider the omnibox being focused in
the triggering logic for reopen tab in-product help.

Bug: 887991
Change-Id: Ie9bac48ebb471002926156c488deb886688a4af4
Reviewed-on: https://chromium-review.googlesource.com/c/1351495Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611487}

chrome/browser/ui/in_product_help/reopen_tab_in_product_help.cc

@@ -50,10 +50,6 @@ void ReopenTabInProductHelp::NewTabOpened() {
   trigger_.NewTabOpened();
 }
 
-void ReopenTabInProductHelp::OmniboxFocused() {
-  trigger_.OmniboxFocused();
-}
-
 void ReopenTabInProductHelp::TabReopened() {
   GetTracker()->NotifyEvent(feature_engagement::events::kTabReopened);
 }

chrome/browser/ui/in_product_help/reopen_tab_in_product_help.h

@@ -35,12 +35,9 @@ class ReopenTabInProductHelp : public BrowserListObserver, public KeyedService {
   ReopenTabInProductHelp(Profile* profile, const base::TickClock* clock);
   ~ReopenTabInProductHelp() override;
 
-  // Should be called when the user opens a blank new tab.
-  void NewTabOpened();
-
-  // Should be called when the user focuses on the omnibox. Possibly triggers
+  // Should be called when the user opens a blank new tab. Possibly triggers
   // IPH.
-  void OmniboxFocused();
+  void NewTabOpened();
 
   // Should be called when the user reopens a previously closed tab, either
   // through CTRL+SHIFT+T or through the recent tabs menu.

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_trigger.cc

@@ -16,9 +16,6 @@ const base::TimeDelta ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration =
 // static
 const base::TimeDelta ReopenTabInProductHelpTrigger::kNewTabOpenedTimeout =
     base::TimeDelta::FromSeconds(10);
-// static
-const base::TimeDelta ReopenTabInProductHelpTrigger::kOmniboxFocusedTimeout =
-    base::TimeDelta::FromSeconds(10);
 
 ReopenTabInProductHelpTrigger::ReopenTabInProductHelpTrigger(
     feature_engagement::Tracker* tracker,
@@ -27,9 +24,8 @@ ReopenTabInProductHelpTrigger::ReopenTabInProductHelpTrigger(
   DCHECK(tracker);
   DCHECK(clock);
 
-  // Timeouts must be non-zero.
+  // Timeout must be non-zero.
   DCHECK(!kNewTabOpenedTimeout.is_zero());
-  DCHECK(!kOmniboxFocusedTimeout.is_zero());
 }
 
 ReopenTabInProductHelpTrigger::~ReopenTabInProductHelpTrigger() = default;
@@ -61,26 +57,14 @@ void ReopenTabInProductHelpTrigger::NewTabOpened() {
   const base::TimeDelta elapsed_time = clock_->NowTicks() - time_of_last_step_;
 
   if (elapsed_time < kNewTabOpenedTimeout) {
-    trigger_state_ = NEW_TAB_OPENED;
-    time_of_last_step_ = clock_->NowTicks();
-  } else {
-    ResetTriggerState();
-  }
-}
-
-void ReopenTabInProductHelpTrigger::OmniboxFocused() {
-  if (trigger_state_ != NEW_TAB_OPENED)
-    return;
-
-  const base::TimeDelta elapsed_time = clock_->NowTicks() - time_of_last_step_;
-
-  if (elapsed_time < kOmniboxFocusedTimeout) {
     tracker_->NotifyEvent(feature_engagement::events::kReopenTabConditionsMet);
     if (tracker_->ShouldTriggerHelpUI(
             feature_engagement::kIPHReopenTabFeature)) {
       DCHECK(cb_);
       cb_.Run();
     }
+  } else {
+    ResetTriggerState();
   }
 }
 

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_trigger.h

@@ -37,13 +37,10 @@ class ReopenTabInProductHelpTrigger {
   // Should be called when an active tab is closed.
   void ActiveTabClosed(base::TimeDelta active_duration);
 
-  // Should be called when a blank new tab is opened by user action.
+  // Should be called when a blank new tab is opened by user action. Possibly
+  // triggers IPH.
   void NewTabOpened();
 
-  // Should be called when the user focuses on the omnibox. Possibly triggers
-  // IPH.
-  void OmniboxFocused();
-
   // Must be called once after IPH finishes. Must only be called after the
   // callback is called.
   void HelpDismissed();
@@ -51,7 +48,6 @@ class ReopenTabInProductHelpTrigger {
   // Timeout constants. Exposed for unit testing.
   static const base::TimeDelta kTabMinimumActiveDuration;
   static const base::TimeDelta kNewTabOpenedTimeout;
-  static const base::TimeDelta kOmniboxFocusedTimeout;
 
  private:
   // Sets state as if user has not performed any actions.
@@ -65,7 +61,6 @@ class ReopenTabInProductHelpTrigger {
   enum TriggerState {
     NO_ACTIONS_SEEN,
     ACTIVE_TAB_CLOSED,
-    NEW_TAB_OPENED,
   };
 
   TriggerState trigger_state_;

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_trigger_unittest.cc

@@ -57,7 +57,6 @@ TEST(ReopenTabInProductHelpTriggerTest, TriggersIPH) {
   reopen_tab_iph.ActiveTabClosed(
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 }
 
 TEST(ReopenTabInProductHelpTriggerTest, RespectsBackendShouldTrigger) {
@@ -77,7 +76,6 @@ TEST(ReopenTabInProductHelpTriggerTest, RespectsBackendShouldTrigger) {
   reopen_tab_iph.ActiveTabClosed(
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 }
 
 TEST(ReopenTabInProductHelpTriggerTest, TabNotActiveLongEnough) {
@@ -92,10 +90,9 @@ TEST(ReopenTabInProductHelpTriggerTest, TabNotActiveLongEnough) {
   reopen_tab_iph.ActiveTabClosed(
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration / 2);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 }
 
-TEST(ReopenTabInProductHelpTriggerTest, RespectsTimeouts) {
+TEST(ReopenTabInProductHelpTriggerTest, RespectsTimeout) {
   NiceMock<MockTracker> mock_tracker;
 
   EXPECT_CALL(mock_tracker, NotifyEvent(_)).Times(0);
@@ -111,13 +108,6 @@ TEST(ReopenTabInProductHelpTriggerTest, RespectsTimeouts) {
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
   clock.Advance(ReopenTabInProductHelpTrigger::kNewTabOpenedTimeout);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
-
-  reopen_tab_iph.ActiveTabClosed(
-      ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
-  reopen_tab_iph.NewTabOpened();
-  clock.Advance(ReopenTabInProductHelpTrigger::kOmniboxFocusedTimeout);
-  reopen_tab_iph.OmniboxFocused();
 }
 
 TEST(ReopenTabInProductHelpTriggerTest, TriggersTwice) {
@@ -142,7 +132,6 @@ TEST(ReopenTabInProductHelpTriggerTest, TriggersTwice) {
   reopen_tab_iph.ActiveTabClosed(
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 
   EXPECT_TRUE(triggered);
   triggered = false;
@@ -150,7 +139,6 @@ TEST(ReopenTabInProductHelpTriggerTest, TriggersTwice) {
   reopen_tab_iph.ActiveTabClosed(
       ReopenTabInProductHelpTrigger::kTabMinimumActiveDuration);
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 
   EXPECT_TRUE(triggered);
 }

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_unittest.cc

@@ -74,5 +74,4 @@ TEST_F(ReopenTabInProductHelpTest, TriggersIPH) {
   tab_strip_model->CloseSelectedTabs();
 
   reopen_tab_iph.NewTabOpened();
-  reopen_tab_iph.OmniboxFocused();
 }

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -17,8 +17,6 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/in_product_help/reopen_tab_in_product_help.h"
-#include "chrome/browser/ui/in_product_help/reopen_tab_in_product_help_factory.h"
 #include "chrome/browser/ui/omnibox/clipboard_utils.h"
 #include "chrome/browser/ui/omnibox/omnibox_theme.h"
 #include "chrome/browser/ui/view_ids.h"
@@ -1135,22 +1133,17 @@ void OmniboxViewViews::OnFocus() {
     location_bar_view_->Layout();
 
 #if BUILDFLAG(ENABLE_DESKTOP_IN_PRODUCT_HELP)
-  if (location_bar_view_) {
-    // The user must be starting a session in the same tab as a previous one in
-    // order to display the new tab in-product help promo.  While focusing the
-    // omnibox is not always a precursor to starting a new session, we don't
-    // want to wait until the user is in the middle of editing or navigating,
-    // because we'd like to show them the promo at the time when it would be
-    // immediately useful.
-    if (controller()->GetLocationBarModel()->ShouldDisplayURL()) {
-      feature_engagement::NewTabTrackerFactory::GetInstance()
-          ->GetForProfile(location_bar_view_->profile())
-          ->OnOmniboxFocused();
-    }
-
-    auto* reopen_tab_iph = ReopenTabInProductHelpFactory::GetForProfile(
-        location_bar_view_->profile());
-    reopen_tab_iph->OmniboxFocused();
+  // The user must be starting a session in the same tab as a previous one in
+  // order to display the new tab in-product help promo.  While focusing the
+  // omnibox is not always a precursor to starting a new session, we don't
+  // want to wait until the user is in the middle of editing or navigating,
+  // because we'd like to show them the promo at the time when it would be
+  // immediately useful.
+  if (location_bar_view_ &&
+      controller()->GetLocationBarModel()->ShouldDisplayURL()) {
+    feature_engagement::NewTabTrackerFactory::GetInstance()
+        ->GetForProfile(location_bar_view_->profile())
+        ->OnOmniboxFocused();
   }
 #endif