Buffer UI navigation events in DataUseTabModel until rule fetch

chrome/browser/android/data_usage/external_data_use_observer_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/data_usage/external_data_use_observer.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
@@ skipping 55 matching lines @@
         new data_usage::DataUseAggregator(nullptr, nullptr));
     external_data_use_observer_.reset(new ExternalDataUseObserver(
         data_use_aggregator_.get(), io_task_runner_.get(),
         ui_task_runner_.get()));
     // Wait for |external_data_use_observer_| to create the Java object.
     base::RunLoop().RunUntilIdle();
     external_data_use_observer()
         ->data_use_tab_model_->is_control_app_installed_ = true;
   }
 
+  void ReplaceExternalDataUseObserver() {
+    external_data_use_observer_.reset(new ExternalDataUseObserver(
+        data_use_aggregator_.get(), io_task_runner_.get(),
+        ui_task_runner_.get()));
+    // Wait for |external_data_use_observer_| to create the Java object.
+    base::RunLoop().RunUntilIdle();
+  }
+
   // Replaces |external_data_use_observer_| with a new ExternalDataUseObserver.
-  void ReplaceExternalDataUseObserver(
+  void ReplaceExternalDataUseObserverWithVariation(
       std::map<std::string, std::string> variation_params) {
     variations::testing::ClearAllVariationParams();
     ASSERT_TRUE(variations::AssociateVariationParams(
         ExternalDataUseObserver::kExternalDataUseObserverFieldTrial, "Enabled",
         variation_params));
 
     base::FieldTrialList field_trial_list(nullptr);
 
     base::FieldTrialList::CreateFieldTrial(
         ExternalDataUseObserver::kExternalDataUseObserverFieldTrial, "Enabled");
-
-    external_data_use_observer_.reset(new ExternalDataUseObserver(
-        data_use_aggregator_.get(), io_task_runner_.get(),
-        ui_task_runner_.get()));
-    // Wait for |external_data_use_observer_| to create the Java object.
-    base::RunLoop().RunUntilIdle();
+    ReplaceExternalDataUseObserver();
   }
 
   void FetchMatchingRulesDone(const std::vector<std::string>& app_package_name,
                               const std::vector<std::string>& domain_path_regex,
                               const std::vector<std::string>& label) {
     external_data_use_observer_->GetDataUseTabModel()->RegisterURLRegexes(
         app_package_name, domain_path_regex, label);
     base::RunLoop().RunUntilIdle();
   }
 
@@ skipping 279 matching lines @@
 }
 
 // Tests the matching rule fetch behavior when the external control app is
 // installed and not installed. Matching rules should be fetched when control
 // app gets installed. If control app is installed and no valid rules are found,
 // matching rules are fetched on every navigation. Rules are not fetched if
 // control app is not installed  or if more than zero valid rules have been
 // fetched.
 TEST_F(ExternalDataUseObserverTest, MatchingRuleFetchOnControlAppInstall) {
   {
-    // Matching rules not fetched on navigation if control app is not installed.
+    // Matching rules not fetched on navigation if control app is not installed,
+    // and navigation events will be buffered.
     external_data_use_observer()->last_matching_rules_fetch_time_ =
         base::TimeTicks();
     external_data_use_observer()
         ->data_use_tab_model_->is_control_app_installed_ = false;
     base::HistogramTester histogram_tester;
     external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
         kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
         std::string());
     base::RunLoop().RunUntilIdle();
     histogram_tester.ExpectTotalCount("DataUsage.MatchingRulesCount.Valid", 0);
+    EXPECT_EQ(1, external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_->size());
+    external_data_use_observer()
+        ->data_use_tab_model_->data_use_ui_navigations_->clear();
   }
 
   {
-    // Matching rules are fetched when control app is installed.
+    // Matching rules are fetched  when control app is installed.

[tbansal1, 2016/03/29 16:20:41]

extra space?

[Raj, 2016/03/29 18:03:21]

Done.

     base::HistogramTester histogram_tester;
     external_data_use_observer()
         ->data_use_tab_model_->OnControlAppInstallStateChange(true);
     base::RunLoop().RunUntilIdle();
     histogram_tester.ExpectTotalCount("DataUsage.MatchingRulesCount.Valid", 1);
+    EXPECT_FALSE(external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_.get());
   }
 
   {
     // Matching rules fetched on every navigation if control app is installed
     // and zero rules are available.
     external_data_use_observer()->last_matching_rules_fetch_time_ =
         base::TimeTicks();
     base::HistogramTester histogram_tester;
     external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
         kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
@@ skipping 155 matching lines @@
   const int kDefaultMaxDataReportSubmitWaitMsec = 20000;
   const int64_t kDataUseReportMinBytes = 5000;
   variation_params["fetch_matching_rules_duration_seconds"] =
       base::Int64ToString(kFetchMatchingRulesDurationSeconds);
   variation_params["data_report_submit_timeout_msec"] =
       base::Int64ToString(kDefaultMaxDataReportSubmitWaitMsec);
   variation_params["data_use_report_min_bytes"] =
       base::Int64ToString(kDataUseReportMinBytes);
 
   // Create another ExternalDataUseObserver object.
-  ReplaceExternalDataUseObserver(variation_params);
+  ReplaceExternalDataUseObserverWithVariation(variation_params);
   EXPECT_EQ(base::TimeDelta::FromSeconds(kFetchMatchingRulesDurationSeconds),
             external_data_use_observer()->fetch_matching_rules_duration_);
   EXPECT_EQ(
       base::TimeDelta::FromMilliseconds(kDefaultMaxDataReportSubmitWaitMsec),
       external_data_use_observer()->data_report_submit_timeout_);
   EXPECT_EQ(kDataUseReportMinBytes,
             external_data_use_observer()->data_use_report_min_bytes_);
 }
 
 // Tests if the metrics are recorded correctly.
 TEST_F(ExternalDataUseObserverTest, DataUseReportTimedOut) {
   base::HistogramTester histogram_tester;
   std::map<std::string, std::string> variation_params;
   variation_params["data_report_submit_timeout_msec"] = "0";
   variation_params["data_use_report_min_bytes"] = "0";
 
   // Create another ExternalDataUseObserver object.
-  ReplaceExternalDataUseObserver(variation_params);
+  ReplaceExternalDataUseObserverWithVariation(variation_params);
   histogram_tester.ExpectTotalCount(kUMAMatchingRuleFirstFetchDurationHistogram,
                                     0);
 
   // Trigger the control app install, and matching rules will be fetched.
   external_data_use_observer()
       ->GetDataUseTabModel()
       ->OnControlAppInstallStateChange(true);
   base::RunLoop().RunUntilIdle();
   histogram_tester.ExpectTotalCount(kUMAMatchingRuleFirstFetchDurationHistogram,
                                     1);
@@ skipping 14 matching lines @@
   // First data use report should be marked as timed out.
   histogram_tester.ExpectUniqueSample(
       "DataUsage.ReportSubmissionResult",
       ExternalDataUseObserver::DATAUSAGE_REPORT_SUBMISSION_TIMED_OUT, 1);
   histogram_tester.ExpectUniqueSample(
       "DataUsage.ReportSubmission.Bytes.TimedOut",
       default_upload_bytes() + default_download_bytes(), 1);
   histogram_tester.ExpectTotalCount(kUMAReportSubmissionDurationHistogram, 0);
 }
 
+// Tests the buffering of UI navigation events until matching rule fetch is
+// complete, or until the external control app not installed callback is
+// received, or until a maximum number of events are buffered, whichever comes
+// first.
+TEST_F(ExternalDataUseObserverTest, ProcessBufferedNavigationEvents) {

[tbansal1, 2016/03/29 16:20:41]

 Seems like this is essentially 3 different tests. Can this be split into 3
different tests?

[tbansal1, 2016/03/29 16:20:41]

This test does not really check if ProcessBufferedNavigations did something
useful or not. May be instead of checking the size(), you can see if
ProcessBufferedNavigations actually changed the state of the tab or not.

[Raj, 2016/03/29 18:03:21]

I have added a unittest in data_use_tab_model_unittest.cc to check if tracking
starts for the buffered events.

[Raj, 2016/03/29 18:03:21]

Done.

+  const uint32_t kDefaultMaxNavigationEventsBuffered = 100;
+  {
+    // Navigation events will be buffered until control app not installed
+    // callback is received.
+    external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+        kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+        std::string());
+    external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+        kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+        std::string());
+    EXPECT_EQ(2, external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_->size());
+    external_data_use_observer()
+        ->data_use_tab_model_->OnControlAppInstallStateChange(false);
+    base::RunLoop().RunUntilIdle();
+    EXPECT_FALSE(external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_.get());
+  }
+
+  {
+    // Navigation events will be buffered until control app is installed and
+    // matching rules are fetched.
+    ReplaceExternalDataUseObserver();
+    base::HistogramTester histogram_tester;
+    external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+        kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+        std::string());
+    external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+        kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+        std::string());
+    EXPECT_EQ(2, external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_->size());
+    external_data_use_observer()
+        ->data_use_tab_model_->OnControlAppInstallStateChange(true);
+    base::RunLoop().RunUntilIdle();
+    histogram_tester.ExpectTotalCount("DataUsage.MatchingRulesCount.Valid", 3);
+    EXPECT_FALSE(external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_.get());
+  }
+  {
+    // Navigation events will be buffered until the buffer reaches a maximum
+    // limit.
+    ReplaceExternalDataUseObserver();
+    for (size_t count = 1; count < kDefaultMaxNavigationEventsBuffered;

[tbansal1, 2016/03/29 16:20:41]

this is flaky. If somebody changes value at one place, this test will fail. If
you want to know the value of this outside the class, pass this as a parameter
in the constructor of DataUseTabModel.

[Raj, 2016/03/29 18:03:21]

I do not want to add this limit as another parameter to DataUseTabModel. I have
used a higher number in the testfile and expecting that the buffer will get
cleared eventually at some point (not at an exact size limit). WDYT.

+         count++) {

[tbansal1, 2016/03/29 16:20:41]

++count

[Raj, 2016/03/29 18:03:21]

Done.

+      external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+          kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+          std::string());
+      EXPECT_EQ(i, external_data_use_observer()
+                       ->data_use_tab_model_->data_use_ui_navigations_->size());
+    }
+    // Next navigation event will trigger clearing of the buffer.
+    external_data_use_observer()->data_use_tab_model_->OnNavigationEvent(
+        kDefaultTabId, DataUseTabModel::TRANSITION_LINK, GURL(kDefaultURL),
+        std::string());
+    EXPECT_FALSE(external_data_use_observer()
+                     ->data_use_tab_model_->data_use_ui_navigations_.get());
+  }
+}
+
 }  // namespace android
 
 }  // namespace chrome