Add UMA histograms for data usage tab model

chrome/browser/android/data_usage/data_use_tab_model.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/data_use_tab_model.h"
 
+#include "base/metrics/histogram_macros.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"
 
 namespace {
 
 // TODO(rajendrant): To be changeable via field trial.
 // Indicates the maximum number of tabs to maintain session information about.
 const size_t kMaxTabEntries = 200;
 
 // Returns true if |tab_id| is a valid tab ID.
 bool IsValidTabID(int32_t tab_id) {
   return tab_id >= 0;
 }
 
+const char kUMAExpiredInactiveTabEntryRemovaltimeSecondsHistogram[] =

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

May be remove Seconds from the variable names (if you end up using
UMA_HISTOGRAM_TIMES)?

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

Not sure. Each UMA needs different precision.

+    "DataUse.TabModel.ExpiredInactiveTabEntryRemovaltime";
+const char kUMAExpiredActiveTabEntryRemovaltimeHoursHistogram[] =
+    "DataUse.TabModel.ExpiredActiveTabEntryRemovaltime";
+const char kUMAUnexpiredTabEntryRemovaltimeMinutesHistogram[] =
+    "DataUse.TabModel.UnexpiredTabEntryLifetime";
+
 }  // namespace
 
 namespace chrome {
 
 namespace android {
 
 size_t DataUseTabModel::GetMaxTabEntriesForTests() {
   return kMaxTabEntries;
 }
 
 DataUseTabModel::DataUseTabModel(
     const ExternalDataUseObserver* data_use_observer,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
     : data_use_observer_(data_use_observer),
       observer_list_(new base::ObserverListThreadSafe<TabDataUseObserver>),
       weak_factory_(this) {}
 
 DataUseTabModel::~DataUseTabModel() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
+base::TimeTicks DataUseTabModel::Now() const {

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

Follow the same order as in .h file. Declare before  GetMaxTabEntriesForTests().

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

Done.

+  return base::TimeTicks::Now();
+}
+
 base::WeakPtr<DataUseTabModel> DataUseTabModel::GetWeakPtr() {
   DCHECK(thread_checker_.CalledOnValidThread());
   return weak_factory_.GetWeakPtr();
 }
 
 void DataUseTabModel::OnNavigationEvent(int32_t tab_id,
                                         TransitionType transition,
                                         const GURL& url,
                                         const std::string& package) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 108 matching lines @@
   if (tab_entry_iterator != active_tabs_.end() &&
       tab_entry_iterator->second.EndTracking()) {
     NotifyObserversOfTrackingEnding(tab_id);
   }
 }
 
 void DataUseTabModel::CompactTabEntries() {
   // Remove expired tab entries.
   for (TabEntryMap::iterator tab_entry_iterator = active_tabs_.begin();
        tab_entry_iterator != active_tabs_.end();) {
-    if (tab_entry_iterator->second.IsExpired())
-      active_tabs_.erase(tab_entry_iterator++);
-    else
-      ++tab_entry_iterator;
+    const auto& tab_entry = tab_entry_iterator->second;
+    if (tab_entry.IsExpired()) {
+      // Track the lifetime of expired tab entry.
+      const base::TimeDelta removal_time =
+          Now() - tab_entry.GetLatestStartOrEndTime();
+      if (!tab_entry.IsTrackingDataUse()) {
+        UMA_HISTOGRAM_COUNTS_1000(

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

Why not UMA_HISTOGRAM_TIMES, or its sibling UMA_HISTOGRAM_LONG_TIMES etc.?
Here, and everywhere else in this CL?

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

Tracking expired inactive tab entry removal times in the range 1 second to 16.67
minutes.

UMA_HISTOGRAM_TIMES and its variations track in milliseconds and spend a lot of
buckets in milliseconds.
For tab model millisecond tracking is not needed.
So following examples similar to 'Ash.TouchView.TouchViewInactiveTotal', etc.

[tbansal1, 2015/11/16 21:09:13]

 UMA_HISTOGRAM_CUSTOM_TIMES?

[Raj, 2015/11/17 07:04:03]

Done.

+            kUMAExpiredInactiveTabEntryRemovaltimeSecondsHistogram,
+            base::TimeDelta(removal_time).InSeconds());
+      } else {
+        UMA_HISTOGRAM_CUSTOM_COUNTS(

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

Why not UMA_HISTOGRAM_TIMES?

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

Tracking expired active tab entry removal times in the range 1 hour to 5 days.
5 days is the default max removal time - kOpenTabExpirationDurationSeconds. So
this UMA could help tune that parameter via field-trial.

+            kUMAExpiredActiveTabEntryRemovaltimeHoursHistogram,
+            base::TimeDelta(removal_time).InHours(), 1, 120, 50);
+      }
+      active_tabs_.erase(tab_entry_iterator);

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

Is this correct? Documentation for erase() says that iterator is invalidated
after element is erased.

I think what you want to do is
if(...isExpired) {
  ....
  tab_entry_iterator = active_tabs_.erase(tab_entry_iterator);
} else {
 ++tab_entry_iterator;
}

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

Done.

+    }
+    ++tab_entry_iterator;
   }
 
   if (active_tabs_.size() <= kMaxTabEntries)
     return;
 
   // Remove oldest unexpired tab entries.
   while (active_tabs_.size() > kMaxTabEntries) {
     // Find oldest tab entry.
     TabEntryMap::iterator oldest_tab_entry_iterator = active_tabs_.begin();
     for (TabEntryMap::iterator tab_entry_iterator = active_tabs_.begin();
          tab_entry_iterator != active_tabs_.end(); ++tab_entry_iterator) {
       if (oldest_tab_entry_iterator->second.GetLatestStartOrEndTime() >
           tab_entry_iterator->second.GetLatestStartOrEndTime()) {
         oldest_tab_entry_iterator = tab_entry_iterator;
       }
     }
     DCHECK(oldest_tab_entry_iterator != active_tabs_.end());
+    UMA_HISTOGRAM_COUNTS_1000(

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

Why not UMA_HISTOGRAM_TIMES?

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

Tracking unexpired tab entry removal times in the range 1 minute to 16.67 hours.

+        kUMAUnexpiredTabEntryRemovaltimeMinutesHistogram,
+        base::TimeDelta(
+            Now() - oldest_tab_entry_iterator->second.GetLatestStartOrEndTime())
+            .InMinutes());
     active_tabs_.erase(oldest_tab_entry_iterator);
   }
 }
 
 }  // namespace android
 
 }  // namespace chrome