Always call into in-product help backend upon new tab opening

This makes testing the UI easier: in in-product help demo mode, the
first call to ShouldTriggerHelpUI() will always return
true. Previously to test the UI you still had to meet the triggering
conditions handled by ReopenTabInProductHelpTrigger, which meant
leaving a tab open for a duration of time. Now in demo mode the UI
should always show upon opening a new tab.

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

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_trigger.cc

@@ -83,21 +83,25 @@ void ReopenTabInProductHelpTrigger::ActiveTabClosed(
 }
 
 void ReopenTabInProductHelpTrigger::NewTabOpened() {
-  if (trigger_state_ != ACTIVE_TAB_CLOSED)
-    return;
-
   const base::TimeDelta elapsed_time = clock_->NowTicks() - time_of_last_step_;
 
-  if (elapsed_time < new_tab_opened_timeout_) {
+  if (trigger_state_ == ACTIVE_TAB_CLOSED &&
+      elapsed_time < new_tab_opened_timeout_) {
     tracker_->NotifyEvent(feature_engagement::events::kReopenTabConditionsMet);
-    if (tracker_->ShouldTriggerHelpUI(
-            feature_engagement::kIPHReopenTabFeature)) {
-      DCHECK(cb_);
-      cb_.Run();
-    }
   } else {
     ResetTriggerState();
   }
+
+  // Make sure ShouldTriggerHelpUI is always called when a new tab is
+  // opened. This makes testing the UI easier when the feature engagement demo
+  // mode is enabled (in which the first call always returns true). Making the
+  // call not conditional on our triggering conditions means the UI can be
+  // triggered more easily and consistently. In production, this will always
+  // return false anyway since we didn't call NotifyEvent().
+  if (tracker_->ShouldTriggerHelpUI(feature_engagement::kIPHReopenTabFeature)) {
+    DCHECK(cb_);
+    cb_.Run();
+  }
 }
 
 void ReopenTabInProductHelpTrigger::HelpDismissed() {

chrome/browser/ui/in_product_help/reopen_tab_in_product_help_trigger_unittest.cc

@@ -107,7 +107,6 @@ TEST_F(ReopenTabInProductHelpTriggerTest, TabNotActiveLongEnough) {
   NiceMock<MockTracker> mock_tracker;
 
   EXPECT_CALL(mock_tracker, NotifyEvent(_)).Times(0);
-  EXPECT_CALL(mock_tracker, ShouldTriggerHelpUI(_)).Times(0);
 
   base::SimpleTestTickClock clock;
   ReopenTabInProductHelpTrigger reopen_tab_iph(&mock_tracker, &clock);
@@ -120,7 +119,6 @@ TEST_F(ReopenTabInProductHelpTriggerTest, RespectsTimeout) {
   NiceMock<MockTracker> mock_tracker;
 
   EXPECT_CALL(mock_tracker, NotifyEvent(_)).Times(0);
-  EXPECT_CALL(mock_tracker, ShouldTriggerHelpUI(_)).Times(0);
 
   base::SimpleTestTickClock clock;
   ReopenTabInProductHelpTrigger reopen_tab_iph(&mock_tracker, &clock);
@@ -163,3 +161,31 @@ TEST_F(ReopenTabInProductHelpTriggerTest, TriggersTwice) {
 
   EXPECT_TRUE(triggered);
 }
+
+// Ensures backend's ShouldTriggerHelpUI() is called whenever a new tab is
+// opened, even if we haven't met our triggering conditions yet.
+TEST_F(ReopenTabInProductHelpTriggerTest, AlwaysCallsBackendOnNewTab) {
+  NiceMock<MockTracker> mock_tracker;
+
+  EXPECT_CALL(
+      mock_tracker,
+      NotifyEvent(Eq(feature_engagement::events::kReopenTabConditionsMet)))
+      .Times(0);
+  EXPECT_CALL(mock_tracker, ShouldTriggerHelpUI(_))
+      .Times(2)
+      .WillRepeatedly(Return(false));
+  EXPECT_CALL(mock_tracker, Dismissed(_)).Times(0);
+
+  base::SimpleTestTickClock clock;
+  ReopenTabInProductHelpTrigger reopen_tab_iph(&mock_tracker, &clock);
+
+  reopen_tab_iph.SetShowHelpCallback(
+      base::BindRepeating(DismissImmediately, &reopen_tab_iph));
+
+  // Opening a new tab without closing an active tab first:
+  reopen_tab_iph.NewTabOpened();
+
+  // Opening a new tab after closing a tab too quickly:
+  reopen_tab_iph.ActiveTabClosed(kTabMinimumActiveDuration / 2);
+  reopen_tab_iph.NewTabOpened();
+}