UMA for the Stale Previews timestamp

chrome/browser/previews/previews_infobar_delegate_unittest.cc

Index: chrome/browser/previews/previews_infobar_delegate_unittest.cc
diff --git a/chrome/browser/previews/previews_infobar_delegate_unittest.cc b/chrome/browser/previews/previews_infobar_delegate_unittest.cc
index 107da1d9e9a50ab2046207a4a3e93e6cc09aefa6..68e9b4c6395cbf9915d80022a4d87e2255fa5c01 100644
--- a/chrome/browser/previews/previews_infobar_delegate_unittest.cc
+++ b/chrome/browser/previews/previews_infobar_delegate_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/previews/previews_infobar_delegate.h"
 
+#include <map>
 #include <memory>
 #include <string>
 
@@ -12,8 +13,10 @@
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_param_associator.h"
 #include "base/metrics/field_trial_params.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -71,6 +74,9 @@ const char kUMAPreviewsInfoBarActionOffline[] =
 const char kUMAPreviewsInfoBarActionLitePage[] =
     "Previews.InfoBarAction.LitePage";
 
+// Key of the UMA Previews.InfoBarTimestamp histogram.
+const char kUMAPreviewsInfoBarTimestamp[] = "Previews.InfoBarTimestamp";
+
 class TestPreviewsWebContentsObserver
     : public content::WebContentsObserver,
       public content::WebContentsUserData<TestPreviewsWebContentsObserver> {
@@ -120,7 +126,8 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(TestPreviewsWebContentsObserver);
 class PreviewsInfoBarDelegateUnitTest : public ChromeRenderViewHostTestHarness {
  protected:
   PreviewsInfoBarDelegateUnitTest()
-      : field_trial_list_(new base::FieldTrialList(nullptr)) {}
+      : field_trial_list_(new base::FieldTrialList(nullptr)),
+        tester_(new base::HistogramTester()) {}
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
@@ -178,14 +185,15 @@ class PreviewsInfoBarDelegateUnitTest : public ChromeRenderViewHostTestHarness {
         infobar_service->infobar_at(0)->delegate());
   }
 
-  void EnableStalePreviewsTimestamp() {
+  void EnableStalePreviewsTimestamp(
+      std::map<std::string, std::string> variation_params) {

[tbansal1, 2017/06/23 00:37:42]

nit: pass as const ref.

[megjablon, 2017/06/23 18:16:14]

Done.

+    field_trial_list_.reset();
+    field_trial_list_.reset(new base::FieldTrialList(nullptr));
+    base::FieldTrialParamAssociator::GetInstance()->ClearAllParamsForTesting();
+
     const std::string kTrialName = "TrialName";
     const std::string kGroupName = "GroupName";
 
-    std::map<std::string, std::string> variation_params;
-    variation_params["min_staleness_in_minutes"] = "2";
-    variation_params["max_staleness_in_minutes"] = "1440";
-
     base::AssociateFieldTrialParams(kTrialName, kGroupName, variation_params);
     base::FieldTrial* field_trial =
         base::FieldTrialList::CreateFieldTrial(kTrialName, kGroupName);
@@ -197,6 +205,23 @@ class PreviewsInfoBarDelegateUnitTest : public ChromeRenderViewHostTestHarness {
     scoped_feature_list_.InitWithFeatureList(std::move(feature_list));
   }
 
+  void TestStalePreviews(
+      int staleness_in_minutes,
+      base::string16 expected_timestamp,
+      PreviewsInfoBarDelegate::PreviewsInfoBarTimestamp expected_bucket) {
+    PreviewsInfoBarDelegate* infobar = CreateInfoBar(
+        previews::PreviewsType::LITE_PAGE,
+        base::Time::Now() - base::TimeDelta::FromMinutes(staleness_in_minutes),
+        true /* is_data_saver_user */);
+    EXPECT_EQ(expected_timestamp, infobar->GetTimestampText());
+    tester_->ExpectBucketCount(kUMAPreviewsInfoBarTimestamp, expected_bucket,
+                               1);
+    // Dismiss the infobar.
+    InfoBarService::FromWebContents(web_contents())->RemoveAllInfoBars(false);
+    PreviewsInfoBarTabHelper::FromWebContents(web_contents())
+        ->set_displayed_preview_infobar(false);
+  }
+
   void OnDismissPreviewsInfobar(bool user_opt_out) {
     user_opt_out_ = user_opt_out;
   }
@@ -211,11 +236,10 @@ class PreviewsInfoBarDelegateUnitTest : public ChromeRenderViewHostTestHarness {
   base::Optional<bool> user_opt_out_;
   std::unique_ptr<base::FieldTrialList> field_trial_list_;
   base::test::ScopedFeatureList scoped_feature_list_;
+  std::unique_ptr<base::HistogramTester> tester_;
 };
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestNavigationDismissal) {
-  base::HistogramTester tester;
-
   CreateInfoBar(previews::PreviewsType::LOFI, base::Time(),
                 true /* is_data_saver_user */);
 
@@ -232,7 +256,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestNavigationDismissal) {
   EXPECT_EQ(0U, infobar_service()->infobar_count());
   EXPECT_FALSE(user_opt_out_.value());
 
-  tester.ExpectBucketCount(
+  tester_->ExpectBucketCount(
       kUMAPreviewsInfoBarActionLoFi,
       PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_NAVIGATION, 1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
@@ -240,8 +264,6 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestNavigationDismissal) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestReloadDismissal) {
-  base::HistogramTester tester;
-
   // Navigate to test URL, so we can reload later.
   NavigateAndCommit(GURL(kTestUrl));
 
@@ -264,9 +286,9 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestReloadDismissal) {
   EXPECT_EQ(0U, infobar_service()->infobar_count());
   EXPECT_FALSE(user_opt_out_.value());
 
-  tester.ExpectBucketCount(kUMAPreviewsInfoBarActionLoFi,
-                           PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_RELOAD,
-                           1);
+  tester_->ExpectBucketCount(
+      kUMAPreviewsInfoBarActionLoFi,
+      PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_RELOAD, 1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiLoadImagesPerSession));
 
@@ -280,8 +302,6 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestReloadDismissal) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestUserDismissal) {
-  base::HistogramTester tester;
-
   ConfirmInfoBarDelegate* infobar =
       CreateInfoBar(previews::PreviewsType::LOFI, base::Time(),
                     true /* is_data_saver_user */);
@@ -291,17 +311,15 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestUserDismissal) {
   infobar_service()->infobar_at(0)->RemoveSelf();
   EXPECT_EQ(0U, infobar_service()->infobar_count());
 
-  tester.ExpectBucketCount(kUMAPreviewsInfoBarActionLoFi,
-                           PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_USER,
-                           1);
+  tester_->ExpectBucketCount(kUMAPreviewsInfoBarActionLoFi,
+                             PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_USER,
+                             1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiLoadImagesPerSession));
   EXPECT_FALSE(user_opt_out_.value());
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestTabClosedDismissal) {
-  base::HistogramTester tester;
-
   CreateInfoBar(previews::PreviewsType::LOFI, base::Time(),
                 true /* is_data_saver_user */);
 
@@ -309,7 +327,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestTabClosedDismissal) {
   infobar_service()->infobar_at(0)->RemoveSelf();
   EXPECT_EQ(0U, infobar_service()->infobar_count());
 
-  tester.ExpectBucketCount(
+  tester_->ExpectBucketCount(
       kUMAPreviewsInfoBarActionLoFi,
       PreviewsInfoBarDelegate::INFOBAR_DISMISSED_BY_TAB_CLOSURE, 1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
@@ -325,6 +343,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLoFi) {
       {true}, {false},
   };
   for (const auto test : tests) {
+    tester_.reset(new base::HistogramTester());
     drp_test_context_->config()->ResetLoFiStatusForTest();
     field_trial_list_.reset();
     field_trial_list_.reset(new base::FieldTrialList(nullptr));
@@ -332,7 +351,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLoFi) {
       base::FieldTrialList::CreateFieldTrial(
           "DataReductionProxyPreviewsBlackListTransition", "Enabled_");
     }
-    base::HistogramTester tester;
+
     // Call Reload and CommitPendingNavigation to force DidFinishNavigation.
     web_contents()->GetController().Reload(content::ReloadType::NORMAL, true);
     content::WebContentsTester::For(web_contents())->CommitPendingNavigation();
@@ -346,7 +365,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLoFi) {
       infobar_service()->infobar_at(0)->RemoveSelf();
     EXPECT_EQ(0U, infobar_service()->infobar_count());
 
-    tester.ExpectBucketCount(
+    tester_->ExpectBucketCount(
         kUMAPreviewsInfoBarActionLoFi,
         PreviewsInfoBarDelegate::INFOBAR_LOAD_ORIGINAL_CLICKED, 1);
     EXPECT_EQ(test.using_previews_blacklist ? 0 : 1,
@@ -364,8 +383,6 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLoFi) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLitePage) {
-  base::HistogramTester tester;
-
   NavigateAndCommit(GURL(kTestUrl));
 
   ConfirmInfoBarDelegate* infobar =
@@ -377,7 +394,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestClickLinkLitePage) {
     infobar_service()->infobar_at(0)->RemoveSelf();
   EXPECT_EQ(0U, infobar_service()->infobar_count());
 
-  tester.ExpectBucketCount(
+  tester_->ExpectBucketCount(
       kUMAPreviewsInfoBarActionLitePage,
       PreviewsInfoBarDelegate::INFOBAR_LOAD_ORIGINAL_CLICKED, 1);
 
@@ -421,14 +438,12 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, InfobarTestShownOncePerNavigation) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, LoFiInfobarTest) {
-  base::HistogramTester tester;
-
   ConfirmInfoBarDelegate* infobar =
       CreateInfoBar(previews::PreviewsType::LOFI, base::Time(),
                     true /* is_data_saver_user */);
 
-  tester.ExpectUniqueSample(kUMAPreviewsInfoBarActionLoFi,
-                            PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
+  tester_->ExpectUniqueSample(kUMAPreviewsInfoBarActionLoFi,
+                              PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
   EXPECT_EQ(1, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiUIShownPerSession));
 
@@ -445,14 +460,12 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, LoFiInfobarTest) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTest) {
-  base::HistogramTester tester;
-
   PreviewsInfoBarDelegate* infobar =
       CreateInfoBar(previews::PreviewsType::LITE_PAGE, base::Time(),
                     true /* is_data_saver_user */);
 
-  tester.ExpectUniqueSample(kUMAPreviewsInfoBarActionLitePage,
-                            PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
+  tester_->ExpectUniqueSample(kUMAPreviewsInfoBarActionLitePage,
+                              PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
   EXPECT_EQ(1, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiUIShownPerSession));
 
@@ -471,14 +484,12 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTest) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarNonDataSaverUserTest) {
-  base::HistogramTester tester;
-
   PreviewsInfoBarDelegate* infobar =
       CreateInfoBar(previews::PreviewsType::OFFLINE, base::Time(),
                     false /* is_data_saver_user */);
 
-  tester.ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
-                            PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
+  tester_->ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
+                              PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiUIShownPerSession));
 
@@ -497,14 +508,12 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarNonDataSaverUserTest) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarDataSaverUserTest) {
-  base::HistogramTester tester;
-
   PreviewsInfoBarDelegate* infobar =
       CreateInfoBar(previews::PreviewsType::OFFLINE, base::Time(),
                     true /* is_data_saver_user */);
 
-  tester.ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
-                            PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
+  tester_->ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
+                              PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
   EXPECT_EQ(0, drp_test_context_->pref_service()->GetInteger(
                    data_reduction_proxy::prefs::kLoFiUIShownPerSession));
 
@@ -523,8 +532,6 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarDataSaverUserTest) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarDisablesLoFi) {
-  base::HistogramTester tester;
-
   TestPreviewsWebContentsObserver::FromWebContents(web_contents())
       ->set_should_have_page_id(false);
 
@@ -534,8 +541,8 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarDisablesLoFi) {
       CreateInfoBar(previews::PreviewsType::OFFLINE, base::Time(),
                     true /* is_data_saver_user */);
 
-  tester.ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
-                            PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
+  tester_->ExpectUniqueSample(kUMAPreviewsInfoBarActionOffline,
+                              PreviewsInfoBarDelegate::INFOBAR_SHOWN, 1);
 
   // Simulate clicking the infobar link.
   if (infobar->LinkClicked(WindowOpenDisposition::CURRENT_TAB))
@@ -558,8 +565,6 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, OfflineInfobarDisablesLoFi) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, PingbackClientClearedTabClosed) {
-  base::HistogramTester tester;
-
   NavigateAndCommit(GURL(kTestUrl));
 
   ConfirmInfoBarDelegate* infobar =
@@ -571,7 +576,7 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, PingbackClientClearedTabClosed) {
     infobar_service()->infobar_at(0)->RemoveSelf();
   EXPECT_EQ(0U, infobar_service()->infobar_count());
 
-  tester.ExpectBucketCount(
+  tester_->ExpectBucketCount(
       kUMAPreviewsInfoBarActionLitePage,
       PreviewsInfoBarDelegate::INFOBAR_LOAD_ORIGINAL_CLICKED, 1);
 
@@ -609,46 +614,72 @@ TEST_F(PreviewsInfoBarDelegateUnitTest, PingbackClientClearedTabClosed) {
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTimestampMintuesTest) {

[tbansal1, 2017/06/23 00:37:42]

s/Mintues/Minutes/

[megjablon, 2017/06/23 18:16:14]

Done.

-  EnableStalePreviewsTimestamp();
-
+  // Use default params.
+  std::map<std::string, std::string> variation_params;
+  EnableStalePreviewsTimestamp(variation_params);
   int staleness_in_minutes = 5;
 
-  PreviewsInfoBarDelegate* infobar = CreateInfoBar(
-      previews::PreviewsType::LITE_PAGE,
-      base::Time::Now() - base::TimeDelta::FromMinutes(staleness_in_minutes),
-      true /* is_data_saver_user */);
-
-  ASSERT_EQ(
+  TestStalePreviews(
+      staleness_in_minutes,
       l10n_util::GetStringFUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_MINUTES,
                                  base::IntToString16(staleness_in_minutes)),
-      infobar->GetTimestampText());
+      PreviewsInfoBarDelegate::TIMESTAMP_SHOWN);
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTimestampHourTest) {
-  EnableStalePreviewsTimestamp();
-
+  // Use default variation_params.
+  std::map<std::string, std::string> variation_params;
+  EnableStalePreviewsTimestamp(variation_params);
   int staleness_in_minutes = 65;
 
-  PreviewsInfoBarDelegate* infobar = CreateInfoBar(
-      previews::PreviewsType::LITE_PAGE,
-      base::Time::Now() - base::TimeDelta::FromMinutes(staleness_in_minutes),
-      true /* is_data_saver_user */);
-
-  ASSERT_EQ(l10n_util::GetStringUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_ONE_HOUR),
-            infobar->GetTimestampText());
+  TestStalePreviews(
+      staleness_in_minutes,
+      l10n_util::GetStringUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_ONE_HOUR),
+      PreviewsInfoBarDelegate::TIMESTAMP_SHOWN);
 }
 
 TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTimestampHoursTest) {
-  EnableStalePreviewsTimestamp();
-
+  // Use default variation_params.
+  std::map<std::string, std::string> variation_params;
+  EnableStalePreviewsTimestamp(variation_params);
   int staleness_in_hours = 2;
 
-  PreviewsInfoBarDelegate* infobar = CreateInfoBar(
-      previews::PreviewsType::LITE_PAGE,
-      base::Time::Now() - base::TimeDelta::FromHours(staleness_in_hours),
-      true /* is_data_saver_user */);
+  TestStalePreviews(
+      staleness_in_hours * 60,
+      l10n_util::GetStringFUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_HOURS,
+                                 base::IntToString16(staleness_in_hours)),
+      PreviewsInfoBarDelegate::TIMESTAMP_SHOWN);
+}
+
+TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTimestampFinchParamsUMA) {
+  std::map<std::string, std::string> variation_params;
+  variation_params["min_staleness_in_minutes"] = "1";
+  variation_params["max_staleness_in_minutes"] = "5";
+  EnableStalePreviewsTimestamp(variation_params);
+
+  TestStalePreviews(
+      1,
+      l10n_util::GetStringFUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_MINUTES,
+                                 base::IntToString16(1)),
+      PreviewsInfoBarDelegate::TIMESTAMP_SHOWN);
+
+  TestStalePreviews(
+      6, base::string16(),
+      PreviewsInfoBarDelegate::TIMESTAMP_NOT_SHOWN_STALENESS_GREATER_THAN_MAX);
+}
 
-  ASSERT_EQ(l10n_util::GetStringFUTF16(IDS_PREVIEWS_INFOBAR_TIMESTAMP_HOURS,
-                                       base::IntToString16(staleness_in_hours)),
-            infobar->GetTimestampText());
+TEST_F(PreviewsInfoBarDelegateUnitTest, PreviewInfobarTimestampUMA) {
+  // Use default params.
+  std::map<std::string, std::string> variation_params;
+  EnableStalePreviewsTimestamp(variation_params);
+
+  TestStalePreviews(
+      1, base::string16(),
+      PreviewsInfoBarDelegate::TIMESTAMP_NOT_SHOWN_PREVIEW_NOT_STALE);
+  TestStalePreviews(
+      -1, base::string16(),
+      PreviewsInfoBarDelegate::TIMESTAMP_NOT_SHOWN_STALENESS_NEGATIVE);
+  TestStalePreviews(
+      1441, base::string16(),
+      PreviewsInfoBarDelegate::TIMESTAMP_NOT_SHOWN_STALENESS_GREATER_THAN_MAX);
 }