Add UMA histograms for data usage tab model

chrome/browser/android/data_usage/data_use_tab_model_unittest.cc

Index: chrome/browser/android/data_usage/data_use_tab_model_unittest.cc
diff --git a/chrome/browser/android/data_usage/data_use_tab_model_unittest.cc b/chrome/browser/android/data_usage/data_use_tab_model_unittest.cc
index f2bf80006ca4b630a8cea6537f7ef91393b09110..cd2a2df64278d8014adca690015216a838e78d20 100644
--- a/chrome/browser/android/data_usage/data_use_tab_model_unittest.cc
+++ b/chrome/browser/android/data_usage/data_use_tab_model_unittest.cc
@@ -10,6 +10,7 @@
 
 #include "base/message_loop/message_loop.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/histogram_tester.h"
 #include "base/time/time.h"
 #include "chrome/browser/android/data_usage/external_data_use_observer.h"
 #include "chrome/browser/android/data_usage/tab_data_use_entry.h"
@@ -44,32 +45,34 @@ namespace android {
 
 class DataUseTabModelTest : public testing::Test {
  public:
-  DataUseTabModelTest() : data_use_tab_model_(nullptr, task_runner_.get()) {
+  DataUseTabModelTest() {}
+
+ protected:
+  void SetUp() override {
     // TODO(rajendrant): Create a mock class for ExternalDataUseObserver to
     // spoof the Matches call and test the OnNavigationEvent.
+    data_use_tab_model_.reset(new DataUseTabModel(nullptr, task_runner_.get()));
   }
 
-  void SetUp() override {}
-
   base::SingleThreadTaskRunner* task_runner() { return task_runner_.get(); }
 
   // Returns true if tab entry for |tab_id| exists in |active_tabs_|.
   bool IsTabEntryExists(int32_t tab_id) const {
-    return data_use_tab_model_.active_tabs_.find(tab_id) !=
-           data_use_tab_model_.active_tabs_.end();
+    return data_use_tab_model_->active_tabs_.find(tab_id) !=
+           data_use_tab_model_->active_tabs_.end();
   }
 
   // Checks if there are |expected_size| tab entries being tracked in
   // |active_tabs_|.
   void ExpectTabEntrySize(uint32_t expected_size) const {
-    EXPECT_EQ(expected_size, data_use_tab_model_.active_tabs_.size());
+    EXPECT_EQ(expected_size, data_use_tab_model_->active_tabs_.size());
   }
 
   // Returns true if the tracking session for tab with id |tab_id| is currently
   // active.
   bool IsTrackingDataUse(int32_t tab_id) const {
-    auto tab_entry_iterator = data_use_tab_model_.active_tabs_.find(tab_id);
-    if (tab_entry_iterator == data_use_tab_model_.active_tabs_.end())
+    auto tab_entry_iterator = data_use_tab_model_->active_tabs_.find(tab_id);
+    if (tab_entry_iterator == data_use_tab_model_->active_tabs_.end())
       return false;
     return tab_entry_iterator->second.IsTrackingDataUse();
   }
@@ -110,13 +113,21 @@ class DataUseTabModelTest : public testing::Test {
                                  std::string(), 1000, 1000);
     std::string actual_label;
     bool actual_return =
-        data_use_tab_model_.GetLabelForDataUse(data_use, &actual_label);
+        data_use_tab_model_->GetLabelForDataUse(data_use, &actual_label);
     EXPECT_EQ(expected_return, actual_return);
     EXPECT_EQ(expected_label, actual_label);
   }
 
+  void StartTrackingDataUse(int32_t tab_id, const std::string& label) {

[tbansal1, 2015/11/16 17:17:38]

It would be easier if DataUseTabModelTest was a derived class of
DataUseTabModel. Then you could simply say:
using StartTrackingDataUse;
(Not sure if parameters need to be specified).

[Raj, 2015/11/16 20:09:36]

DataUseTabModelTest derives from testing::Test.
Not sure if its good to derive from DataUseTabModelTest also.
I see all tests in components/data_reduction_proxy derive only from
testing::Test, and have additional classes that derive from needed base class.

These wrapper member functions help avoid the test methods to be
FRIEND_TEST_ALL_PREFIXES of  DataUseTabModel.

+    data_use_tab_model_->StartTrackingDataUse(tab_id, label);
+  }
+
+  void EndTrackingDataUse(int32_t tab_id) {
+    data_use_tab_model_->EndTrackingDataUse(tab_id);
+  }
+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

[tbansal1, 2015/11/16 17:17:38]

#include the file for SingleThreadTaskRunner.

[Raj, 2015/11/16 20:09:36]

Done.

-  DataUseTabModel data_use_tab_model_;
+  scoped_ptr<DataUseTabModel> data_use_tab_model_;
   base::MessageLoop message_loop_;
 };
 
@@ -136,14 +147,14 @@ TEST_F(DataUseTabModelTest, SingleTabTracking) {
   ExpectEmptyDataUseLabel(kTabID1);
   ExpectEmptyDataUseLabel(kTabID2);
 
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
   ExpectTabEntrySize(TabEntrySize::ONE);
 
   EXPECT_TRUE(IsTrackingDataUse(kTabID1));
   ExpectDataUseLabel(kTabID1, kTestLabel1);
   ExpectEmptyDataUseLabel(kTabID2);
 
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
+  EndTrackingDataUse(kTabID1);
   ExpectTabEntrySize(TabEntrySize::ONE);
   EXPECT_FALSE(IsTrackingDataUse(kTabID1));
 }
@@ -156,9 +167,9 @@ TEST_F(DataUseTabModelTest, MultipleTabTracking) {
   ExpectEmptyDataUseLabel(kTabID2);
   ExpectEmptyDataUseLabel(kTabID3);
 
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.StartTrackingDataUse(kTabID2, kTestLabel2);
-  data_use_tab_model_.StartTrackingDataUse(kTabID3, kTestLabel3);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  StartTrackingDataUse(kTabID2, kTestLabel2);
+  StartTrackingDataUse(kTabID3, kTestLabel3);
   ExpectTabEntrySize(TabEntrySize::THREE);
 
   EXPECT_TRUE(IsTrackingDataUse(kTabID1));
@@ -168,9 +179,9 @@ TEST_F(DataUseTabModelTest, MultipleTabTracking) {
   ExpectDataUseLabel(kTabID2, kTestLabel2);
   ExpectDataUseLabel(kTabID3, kTestLabel3);
 
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID2);
-  data_use_tab_model_.EndTrackingDataUse(kTabID3);
+  EndTrackingDataUse(kTabID1);
+  EndTrackingDataUse(kTabID2);
+  EndTrackingDataUse(kTabID3);
   ExpectTabEntrySize(TabEntrySize::THREE);
   EXPECT_FALSE(IsTrackingDataUse(kTabID1));
   EXPECT_FALSE(IsTrackingDataUse(kTabID2));
@@ -192,9 +203,9 @@ TEST_F(DataUseTabModelTest, ObserverStartEndEvents) {
   EXPECT_CALL(mock_observer, NotifyTrackingStarting(kTabID1)).Times(1);
   EXPECT_CALL(mock_observer, NotifyTrackingEnding(kTabID1)).Times(1);
 
-  data_use_tab_model_.AddObserver(&mock_observer);
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
+  data_use_tab_model_->AddObserver(&mock_observer);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  EndTrackingDataUse(kTabID1);
 
   message_loop_.RunUntilIdle();
 }
@@ -206,7 +217,7 @@ TEST_F(DataUseTabModelTest, MultipleObserverMultipleStartEndEvents) {
 
   for (auto& mock_observer : mock_observers) {
     // Add the observer.
-    data_use_tab_model_.AddObserver(&mock_observer);
+    data_use_tab_model_->AddObserver(&mock_observer);
 
     // Expect start and end events for tab ids 1-3.
     EXPECT_CALL(mock_observer, NotifyTrackingStarting(kTabID1)).Times(1);
@@ -218,12 +229,12 @@ TEST_F(DataUseTabModelTest, MultipleObserverMultipleStartEndEvents) {
   }
 
   // Start and end tracking for tab ids 1-3.
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.StartTrackingDataUse(kTabID2, kTestLabel2);
-  data_use_tab_model_.StartTrackingDataUse(kTabID3, kTestLabel3);
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID2);
-  data_use_tab_model_.EndTrackingDataUse(kTabID3);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  StartTrackingDataUse(kTabID2, kTestLabel2);
+  StartTrackingDataUse(kTabID3, kTestLabel3);
+  EndTrackingDataUse(kTabID1);
+  EndTrackingDataUse(kTabID2);
+  EndTrackingDataUse(kTabID3);
 
   message_loop_.RunUntilIdle();
 }
@@ -237,9 +248,9 @@ TEST_F(DataUseTabModelTest, ObserverNotNotifiedAfterRemove) {
   EXPECT_CALL(mock_observer, NotifyTrackingStarting(kTabID1)).Times(1);
   EXPECT_CALL(mock_observer, NotifyTrackingEnding(kTabID1)).Times(1);
 
-  data_use_tab_model_.AddObserver(&mock_observer);
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
+  data_use_tab_model_->AddObserver(&mock_observer);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  EndTrackingDataUse(kTabID1);
 
   message_loop_.RunUntilIdle();
   testing::Mock::VerifyAndClear(&mock_observer);
@@ -248,35 +259,35 @@ TEST_F(DataUseTabModelTest, ObserverNotNotifiedAfterRemove) {
   EXPECT_CALL(mock_observer, NotifyTrackingStarting(kTabID1)).Times(0);
   EXPECT_CALL(mock_observer, NotifyTrackingEnding(kTabID1)).Times(0);
 
-  data_use_tab_model_.RemoveObserver(&mock_observer);
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
+  data_use_tab_model_->RemoveObserver(&mock_observer);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  EndTrackingDataUse(kTabID1);
 
   message_loop_.RunUntilIdle();
 }
 
 // Checks that tab close event updates the close time of the tab entry.
 TEST_F(DataUseTabModelTest, TabCloseEvent) {
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
-  data_use_tab_model_.EndTrackingDataUse(kTabID1);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  EndTrackingDataUse(kTabID1);
 
   ExpectTabEntrySize(TabEntrySize::ONE);
   EXPECT_TRUE(
-      data_use_tab_model_.active_tabs_[kTabID1].tab_close_time_.is_null());
+      data_use_tab_model_->active_tabs_[kTabID1].tab_close_time_.is_null());
 
-  data_use_tab_model_.OnTabCloseEvent(kTabID1);
+  data_use_tab_model_->OnTabCloseEvent(kTabID1);
 
   ExpectTabEntrySize(TabEntrySize::ONE);
   EXPECT_FALSE(
-      data_use_tab_model_.active_tabs_[kTabID1].tab_close_time_.is_null());
+      data_use_tab_model_->active_tabs_[kTabID1].tab_close_time_.is_null());
 }
 
 // Checks that tab close event ends the active tracking session for the tab.
 TEST_F(DataUseTabModelTest, TabCloseEventEndsTracking) {
-  data_use_tab_model_.StartTrackingDataUse(kTabID1, kTestLabel1);
+  StartTrackingDataUse(kTabID1, kTestLabel1);
   EXPECT_TRUE(IsTrackingDataUse(kTabID1));
 
-  data_use_tab_model_.OnTabCloseEvent(kTabID1);
+  data_use_tab_model_->OnTabCloseEvent(kTabID1);
   EXPECT_FALSE(IsTrackingDataUse(kTabID1));
 
   // Future data use object should be labeled as an empty string.
@@ -293,9 +304,9 @@ TEST_F(DataUseTabModelTest, CompactTabEntriesWithinMaxLimit) {
   ExpectTabEntrySize(TabEntrySize::ZERO);
 
   while (tab_id <= max_tab_entries) {
-    std::string tab_label = base::StringPrintf("track_label_%d", tab_id);
-    data_use_tab_model_.StartTrackingDataUse(tab_id, tab_label);
-    data_use_tab_model_.EndTrackingDataUse(tab_id);
+    std::string tab_label = base::StringPrintf("label_%d", tab_id);
+    StartTrackingDataUse(tab_id, tab_label);
+    EndTrackingDataUse(tab_id);
 
     ExpectTabEntrySize(tab_id);
     ++tab_id;
@@ -308,8 +319,8 @@ TEST_F(DataUseTabModelTest, CompactTabEntriesWithinMaxLimit) {
   while (tab_id < max_tab_entries + 10) {
     EXPECT_TRUE(IsTabEntryExists(oldest_tab_id));
     std::string tab_label = base::StringPrintf("label_%d", tab_id);
-    data_use_tab_model_.StartTrackingDataUse(tab_id, tab_label);
-    data_use_tab_model_.EndTrackingDataUse(tab_id);
+    StartTrackingDataUse(tab_id, tab_label);
+    EndTrackingDataUse(tab_id);
 
     // Oldest entry got removed.
     EXPECT_FALSE(IsTabEntryExists(oldest_tab_id));
@@ -324,7 +335,7 @@ TEST_F(DataUseTabModelTest, CompactTabEntriesWithinMaxLimit) {
   while (tab_id < max_tab_entries + 20) {
     EXPECT_TRUE(IsTabEntryExists(oldest_tab_id));
     std::string tab_label = base::StringPrintf("label_%d", tab_id);
-    data_use_tab_model_.StartTrackingDataUse(tab_id, tab_label);
+    StartTrackingDataUse(tab_id, tab_label);
 
     // Oldest entry got removed.
     EXPECT_FALSE(IsTabEntryExists(oldest_tab_id));
@@ -335,6 +346,97 @@ TEST_F(DataUseTabModelTest, CompactTabEntriesWithinMaxLimit) {
   }
 }
 
+// Test version of |MockDataUseTabModel|, which permits mocking of calls to Now.

[tbansal1, 2015/11/16 17:17:38]

I don't understand why this class is needed. Is it not possible to make
DataUseTabModelTest (defined above on Line 46), a derived class of
DataUseTabModel?

Also, mocking has a special meaning in Chromium. "Mocking" term is used when you
are using the framework provided by gmock

[Raj, 2015/11/16 20:09:36]

Removed the word 'mock', since not using gmock here.
Have two classes similar to data_reduction_proxy_debug_ui_manager_unittest.cc
TestDataUseTabModel - derives DataUseTabModel to override Now()
DataUseTabModelTest - derives testing::Test for all tests.

+class MockDataUseTabModel : public DataUseTabModel {
+ public:
+  MockDataUseTabModel(
+      const ExternalDataUseObserver* data_use_observer,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
+      : DataUseTabModel(data_use_observer, ui_task_runner) {}
+
+  ~MockDataUseTabModel() override {}
+
+  void AdvanceTime(base::TimeDelta now_offset) { now_offset_ = now_offset; }
+
+ private:
+  // Returns the mocked time as Now.
+  base::TimeTicks Now() const override {
+    return base::TimeTicks::Now() + now_offset_;
+  }
+
+  // Represents the delta offset to be added to current time that is returned by
+  // Now.
+  base::TimeDelta now_offset_;
+};
+
+class MockDataUseTabModelTest : public DataUseTabModelTest {
+ protected:
+  void SetUp() override {
+    mock_data_use_tab_model_ =
+        new MockDataUseTabModel(nullptr, task_runner_.get());
+    data_use_tab_model_.reset(mock_data_use_tab_model_);
+  }
+  MockDataUseTabModel* mock_data_use_tab_model_;
+};
+
+TEST_F(MockDataUseTabModelTest, ExpiredInactiveTabEntryRemovaltimeHistogram) {
+  const char kUMAExpiredInactiveTabEntryRemovaltimeSecondsHistogram[] =
+      "DataUse.TabModel.ExpiredInactiveTabEntryRemovaltime";
+  base::HistogramTester histogram_tester;
+
+  StartTrackingDataUse(kTabID1, kTestLabel1);
+  EndTrackingDataUse(kTabID1);
+  EXPECT_FALSE(IsTrackingDataUse(kTabID1));
+  mock_data_use_tab_model_->OnTabCloseEvent(kTabID1);
+
+  // Fake tab close time to make it as expired.
+  EXPECT_TRUE(IsTabEntryExists(kTabID1));
+  auto& tab_entry = mock_data_use_tab_model_->active_tabs_[kTabID1];
+  EXPECT_FALSE(tab_entry.tab_close_time_.is_null());
+  tab_entry.tab_close_time_ -= base::TimeDelta::FromSeconds(
+      TabDataUseEntry::GetClosedTabExpirationDurationSecondsForTests() + 1);
+  EXPECT_TRUE(tab_entry.IsExpired());
+
+  // Fast forward 50 seconds.
+  mock_data_use_tab_model_->AdvanceTime(base::TimeDelta::FromSeconds(50));
+
+  mock_data_use_tab_model_->CompactTabEntries();
+  EXPECT_FALSE(IsTabEntryExists(kTabID1));
+
+  histogram_tester.ExpectTotalCount(
+      kUMAExpiredInactiveTabEntryRemovaltimeSecondsHistogram, 1);
+  histogram_tester.ExpectBucketCount(
+      kUMAExpiredInactiveTabEntryRemovaltimeSecondsHistogram, 50, 1);
+}
+
+TEST_F(MockDataUseTabModelTest, UnexpiredTabEntryRemovaltimeHistogram) {
+  const char kUMAUnexpiredTabEntryRemovaltimeMinutesHistogram[] =
+      "DataUse.TabModel.UnexpiredTabEntryLifetime";
+  base::HistogramTester histogram_tester;
+  const size_t max_tab_entries = DataUseTabModel::GetMaxTabEntriesForTests();
+  uint32_t tab_id = 1;
+
+  while (tab_id <= max_tab_entries) {
+    std::string tab_label = base::StringPrintf("label_%d", tab_id);
+    StartTrackingDataUse(tab_id, tab_label);
+    EndTrackingDataUse(tab_id);
+    ++tab_id;
+  }
+
+  // Fast forward 10 minutes.
+  mock_data_use_tab_model_->AdvanceTime(base::TimeDelta::FromMinutes(10));
+
+  // Adding another tab entry triggers CompactTabEntries.
+  std::string tab_label = base::StringPrintf("label_%d", tab_id);
+  StartTrackingDataUse(tab_id, tab_label);
+  EndTrackingDataUse(tab_id);
+
+  histogram_tester.ExpectTotalCount(
+      kUMAUnexpiredTabEntryRemovaltimeMinutesHistogram, 1);
+  histogram_tester.ExpectBucketCount(
+      kUMAUnexpiredTabEntryRemovaltimeMinutesHistogram, 10, 1);
+}
+
 }  // namespace android
 
 }  // namespace chrome