Add histograms for ExternalDataUseObserver

chrome/browser/android/data_usage/external_data_use_observer_unittest.cc

Index: chrome/browser/android/data_usage/external_data_use_observer_unittest.cc
diff --git a/chrome/browser/android/data_usage/external_data_use_observer_unittest.cc b/chrome/browser/android/data_usage/external_data_use_observer_unittest.cc
index 6ffc2dfbfcdbdbf005cb590ed674b12ec511fedd..c56232a7d768005dfb939f387b41838f1a8c8834 100644
--- a/chrome/browser/android/data_usage/external_data_use_observer_unittest.cc
+++ b/chrome/browser/android/data_usage/external_data_use_observer_unittest.cc
@@ -13,6 +13,7 @@
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/test/histogram_tester.h"
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/android/data_usage/data_use_tab_model_test_utils.h"
 #include "components/data_usage/core/data_use.h"
@@ -51,9 +52,14 @@ class ExternalDataUseObserverTest : public testing::Test {
   }
 
   scoped_ptr<ExternalDataUseObserver> Create() const {
-    return scoped_ptr<ExternalDataUseObserver>(new ExternalDataUseObserver(
+    scoped_ptr<ExternalDataUseObserver> observer(new ExternalDataUseObserver(
         data_use_aggregator_.get(), io_task_runner_.get(),
         ui_task_runner_.get()));
+    // |test_data_use_tab_model| is owned by |observer|.
+    TestDataUseTabModel* test_data_use_tab_model(
+        new TestDataUseTabModel(observer.get(), ui_task_runner_.get()));
+    observer->data_use_tab_model_.reset(test_data_use_tab_model);
+    return observer.Pass();
   }
 
   ExternalDataUseObserver* external_data_use_observer() const {
@@ -300,6 +306,7 @@ TEST_F(ExternalDataUseObserverTest, LabelRemoved) {
 
 // Verifies that buffer size does not exceed the specified limit.
 TEST_F(ExternalDataUseObserverTest, BufferSize) {
+  base::HistogramTester histogram_tester;
   const char kLabel[] = "label";
 
   std::vector<std::string> url_regexes;
@@ -335,6 +342,17 @@ TEST_F(ExternalDataUseObserverTest, BufferSize) {
   // Verify the label of the data use report.
   for (const auto& it : buffered_data_reports())
     EXPECT_EQ(kLabel, it.first.label);
+
+  // Verify that metrics were updated correctly for the lost reports.
+  histogram_tester.ExpectTotalCount(
+      "DataUsage.ReportSubmissionResult",
+      ExternalDataUseObserver::kMaxBufferSize - 1);

[sclittle, 2015/12/02 19:07:55]

nit: If these are all the same sample (e.g. the same number of bytes), you could
just use ExpectUniqueSample(histogram_name, bytes_sample, kMaxBufferSize - 1)

[tbansal1, 2015/12/03 03:19:45]

Done.

+  histogram_tester.ExpectBucketCount(
+      "DataUsage.ReportSubmissionResult", 3,
+      ExternalDataUseObserver::kMaxBufferSize - 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUsage.ReportSubmission.Lost.Bytes",
+      ExternalDataUseObserver::kMaxBufferSize - 1);

[sclittle, 2015/12/02 19:07:54]

Can you also test the samples in this histogram?

[tbansal1, 2015/12/03 03:19:45]

Done.

 }
 
 // Tests that buffered data use reports are merged correctly.
@@ -456,7 +474,8 @@ TEST_F(ExternalDataUseObserverTest, MultipleMatchingRules) {
   external_data_use_observer()->FetchMatchingRulesDoneOnIOThread(
       app_package_names, url_regexes, labels);
   EXPECT_EQ(0U, external_data_use_observer()->buffered_data_reports_.size());
-  EXPECT_FALSE(external_data_use_observer()->submit_data_report_pending_);
+  EXPECT_TRUE(
+      external_data_use_observer()->last_data_report_submitted_.is_null());
   EXPECT_FALSE(external_data_use_observer()->matching_rules_fetch_pending_);
 
   // Check |kLabelFoo| matching rule.
@@ -472,7 +491,8 @@ TEST_F(ExternalDataUseObserverTest, MultipleMatchingRules) {
   external_data_use_observer()->OnDataUse(data_foo_2);
 
   // The foo1 report should have been submitted.
-  EXPECT_TRUE(external_data_use_observer()->submit_data_report_pending_);
+  EXPECT_FALSE(
+      external_data_use_observer()->last_data_report_submitted_.is_null());
 
   // Only the foo2 report should be present.
   EXPECT_EQ(1U, buffered_data_reports().size());
@@ -608,18 +628,45 @@ TEST_F(ExternalDataUseObserverTest, BufferDataUseReports) {
     if (i != num_iterations - 1) {
       // Total buffered bytes is less than the minimum threshold. Data use
       // report should not be send.
-      EXPECT_FALSE(external_data_use_observer()->submit_data_report_pending_);
+      EXPECT_TRUE(
+          external_data_use_observer()->last_data_report_submitted_.is_null());
       EXPECT_EQ(static_cast<int64_t>(i + 1),
                 external_data_use_observer()->total_bytes_buffered_ / 1024);
+      EXPECT_EQ(0, external_data_use_observer()->pending_report_bytes_);
 
     } else {
       // Total bytes is at least the minimum threshold. This should trigger
       // submitting of the buffered data use report.
-      EXPECT_TRUE(external_data_use_observer()->submit_data_report_pending_);
+      EXPECT_FALSE(
+          external_data_use_observer()->last_data_report_submitted_.is_null());
       EXPECT_EQ(0, external_data_use_observer()->total_bytes_buffered_);
     }
   }
   EXPECT_EQ(0, external_data_use_observer()->total_bytes_buffered_);
+  EXPECT_EQ(static_cast<int64_t>(num_iterations),
+            external_data_use_observer()->pending_report_bytes_ / 1024);
+
+  base::HistogramTester histogram_tester;
+  external_data_use_observer()->OnReportDataUseDoneOnIOThread(true);
+
+  // Verify that metrics were updated correctly for the report that was
+  // successfully submitted.
+  histogram_tester.ExpectTotalCount("DataUsage.ReportSubmissionResult", 1);
+  histogram_tester.ExpectBucketCount("DataUsage.ReportSubmissionResult", 0, 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUsage.ReportSubmission.Successful.Bytes", 1);

[sclittle, 2015/12/02 19:07:55]

nit: Can you check the sample for this histogram as well? You can just replace
this with a call to ExpectUniqueSample. Same thing everywhere else that you
ExpectTotalCount(name, 1) in tests.

[tbansal1, 2015/12/03 03:19:45]

Done.

+
+  // Verify that metrics were updated correctly for the report that was not
+  // successfully submitted.
+  external_data_use_observer()->OnDataUse(data_usage::DataUse(
+      GURL("http://www.google.com/#q=abc"), base::TimeTicks::Now(), GURL(), 0,
+      net::NetworkChangeNotifier::CONNECTION_UNKNOWN, "mccmnc_foo", 1024 * 1024,
+      0));
+  external_data_use_observer()->OnReportDataUseDoneOnIOThread(false);
+  histogram_tester.ExpectTotalCount("DataUsage.ReportSubmissionResult", 2);
+  histogram_tester.ExpectBucketCount("DataUsage.ReportSubmissionResult", 1, 1);
+  histogram_tester.ExpectTotalCount("DataUsage.ReportSubmission.Failed.Bytes",
+                                    1);
 }
 
 // Tests if the parameters from the field trial are populated correctly.
@@ -633,9 +680,12 @@ TEST_F(ExternalDataUseObserverTest, Variations) {
   std::map<std::string, std::string> variation_params;
 
   const int kFetchMatchingRulesDurationSeconds = 10000;
+  const int kDefaultMaxDataReportSubmitWaitSeconds = 20000;
   const int64_t kDataUseReportMinBytes = 5000;
   variation_params["fetch_matching_rules_duration_seconds"] =
       base::Int64ToString(kFetchMatchingRulesDurationSeconds);
+  variation_params["data_report_submit_timeout_seconds"] =
+      base::Int64ToString(kDefaultMaxDataReportSubmitWaitSeconds);
   variation_params["data_use_report_min_bytes"] =
       base::Int64ToString(kDataUseReportMinBytes);
 
@@ -656,10 +706,58 @@ TEST_F(ExternalDataUseObserverTest, Variations) {
             external_data_use_obsever_with_variations
                 ->fetch_matching_rules_duration_);
   EXPECT_EQ(
+      base::TimeDelta::FromSeconds(kDefaultMaxDataReportSubmitWaitSeconds),
+      external_data_use_obsever_with_variations->data_report_submit_timeout_);
+  EXPECT_EQ(
       kDataUseReportMinBytes,
       external_data_use_obsever_with_variations->data_use_report_min_bytes_);
 }
 
+// Tests if the metrics are recorded correctly.
+TEST_F(ExternalDataUseObserverTest, DataUseReportTimedOut) {
+  variations::testing::ClearAllVariationParams();
+  std::map<std::string, std::string> variation_params;
+  variation_params["data_report_submit_timeout_seconds"] = "0";
+  variation_params["data_use_report_min_bytes"] = "0";
+
+  ASSERT_TRUE(variations::AssociateVariationParams(
+      ExternalDataUseObserver::kExternalDataUseObserverFieldTrial, "Enabled",
+      variation_params));
+
+  base::FieldTrialList field_trial_list(nullptr);
+
+  base::FieldTrialList::CreateFieldTrial(
+      ExternalDataUseObserver::kExternalDataUseObserverFieldTrial, "Enabled");
+
+  // Create another ExternalDataUseObserver object. This would fetch variation
+  // params.
+  scoped_ptr<ExternalDataUseObserver>
+      external_data_use_obsever_with_variations = Create();
+
+  std::vector<std::string> url_regexes;
+  url_regexes.push_back(
+      "http://www[.]google[.]com/#q=.*|https://www[.]google[.]com/#q=.*");
+  external_data_use_obsever_with_variations->FetchMatchingRulesDoneOnIOThread(
+      std::vector<std::string>(url_regexes.size(), std::string()), url_regexes,
+      std::vector<std::string>(url_regexes.size(), "label"));
+
+  base::HistogramTester histogram_tester;
+  external_data_use_obsever_with_variations->data_use_tab_model()
+      ->OnNavigationEvent(0, DataUseTabModel::TRANSITION_OMNIBOX_SEARCH,
+                          GURL("http://www.google.com/#q=abc"), std::string());
+  external_data_use_obsever_with_variations->OnDataUse(data_usage::DataUse(
+      GURL("http://www.google.com/#q=abc"), base::TimeTicks::Now(), GURL(), 0,
+      net::NetworkChangeNotifier::CONNECTION_UNKNOWN, "mccmnc_foo", 1024 * 1024,
+      0));
+  external_data_use_obsever_with_variations->OnDataUse(data_usage::DataUse(
+      GURL("http://www.google.com/#q=abc"), base::TimeTicks::Now(), GURL(), 0,
+      net::NetworkChangeNotifier::CONNECTION_UNKNOWN, "mccmnc_bar", 1024 * 1024,
+      0));
+  histogram_tester.ExpectTotalCount("DataUsage.ReportSubmissionResult", 1);

[sclittle, 2015/12/02 19:07:55]

nit: you could get rid of the ExpectTotalCount here and just use
ExpectUniqueSample below.

[tbansal1, 2015/12/03 03:19:45]

Done.

+  // First data use report should be marked as timed out.
+  histogram_tester.ExpectBucketCount("DataUsage.ReportSubmissionResult", 2, 1);
+}
+
 }  // namespace android
 
 }  // namespace chrome