Cleanup histogram classes mixing SetFlags into FactoryGet arguments

Generic cleanup of histogram class, renaming *FactoryGet to FactoryGet,
along with reformatting.

The macros were cleaned up to use common sub-macros rather than
repeating code as much.

Removed ThreadSafeHistogram (and associated ASSET_HISTOGRAM macros)
since this class was not getting used.

I introduced UMA_HISTOGRAM_ENUMERATION to support the common use
of LinearHistograms to count various enumerated values.

I added a Flags argument to all the FactoryGet routines to help avoid 
needing to call SetFlags each time a new sample is Add()ed.  This also
simplifies the code.

This will all help prepare for a "don't histogram at all" macro setting
so that I can test the impact of the histogram macro calls on
performance (since there are now so many active histograms).

BUG=31206
r=raman.tenneti
Review URL: http://codereview.chromium.org/515033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35295 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 60 insertions, 129 deletions

diff --git a/net/base/connection_type_histograms.cc b/net/base/connection_type_histograms.cc
index 81affa5..7326a92 100644
--- a/net/base/connection_type_histograms.cc
+++ b/net/base/connection_type_histograms.cc
@@ -22,30 +22,20 @@ namespace net {
 // expansion.
 void UpdateConnectionTypeHistograms(ConnectionType type, bool success) {
   static bool had_connection_type[NUM_OF_CONNECTION_TYPES];
-  static scoped_refptr<Histogram> had_histogram =
-      LinearHistogram::LinearHistogramFactoryGet("Net.HadConnectionType2",
-                                                 1, NUM_OF_CONNECTION_TYPES,
-                                                 NUM_OF_CONNECTION_TYPES + 1);
-  static scoped_refptr<Histogram> success_histogram =
-      LinearHistogram::LinearHistogramFactoryGet("Net.ConnectionTypeCount2",
-                                                 1, NUM_OF_CONNECTION_TYPES,
-                                                 NUM_OF_CONNECTION_TYPES + 1);
-  static scoped_refptr<Histogram> failed_histogram =
-      LinearHistogram::LinearHistogramFactoryGet("Net.ConnectionTypeFailCount2",
-                                                 1, NUM_OF_CONNECTION_TYPES,
-                                                 NUM_OF_CONNECTION_TYPES + 1);
 
   if (type >= 0 && type < NUM_OF_CONNECTION_TYPES) {
     if (!had_connection_type[type]) {
       had_connection_type[type] = true;
-      had_histogram->SetFlags(kUmaTargetedHistogramFlag);
-      had_histogram->Add(type);
+      UMA_HISTOGRAM_ENUMERATION("Net.HadConnectionType2",
+          type, NUM_OF_CONNECTION_TYPES);
     }
 
-    Histogram* histogram;
-    histogram = success ? success_histogram.get() : failed_histogram.get();
-    histogram->SetFlags(kUmaTargetedHistogramFlag);
-    histogram->Add(type);
+    if (success)
+      UMA_HISTOGRAM_ENUMERATION("Net.ConnectionTypeCount2",
+          type, NUM_OF_CONNECTION_TYPES);
+    else
+      UMA_HISTOGRAM_ENUMERATION("Net.ConnectionTypeFailCount2",
+          type, NUM_OF_CONNECTION_TYPES);
   } else {
     NOTREACHED();  // Someone's logging an invalid type!
   }
diff --git a/net/base/mime_sniffer.cc b/net/base/mime_sniffer.cc
index df0f171..8e063cb 100644
--- a/net/base/mime_sniffer.cc
+++ b/net/base/mime_sniffer.cc
@@ -213,9 +213,8 @@ static const MagicNumber kSniffableTags[] = {
 static scoped_refptr<Histogram> UMASnifferHistogramGet(const char* name,
                                                        int array_size) {
   scoped_refptr<Histogram> counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          name, 1, array_size - 1, array_size);
-  counter->SetFlags(kUmaTargetedHistogramFlag);
+      LinearHistogram::FactoryGet(name, 1, array_size - 1, array_size,
+      Histogram::kUmaTargetedHistogramFlag);
   return counter;
 }
 
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index 126a1b6..b279749 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -32,11 +32,7 @@ SdchManager* SdchManager::Global() {
 
 // static
 void SdchManager::SdchErrorRecovery(ProblemCodes problem) {
-  static scoped_refptr<Histogram> histogram =
-      LinearHistogram::LinearHistogramFactoryGet("Sdch3.ProblemCodes_4",
-          MIN_PROBLEM_CODE + 1, MAX_PROBLEM_CODE - 1, MAX_PROBLEM_CODE);
-  histogram->SetFlags(kUmaTargetedHistogramFlag);
-  histogram->Add(problem);
+  UMA_HISTOGRAM_ENUMERATION("Sdch3.ProblemCodes_4", problem, MAX_PROBLEM_CODE);
 }
 
 // static
diff --git a/net/disk_cache/histogram_macros.h b/net/disk_cache/histogram_macros.h
index 6d63dc8..17cd345 100644
--- a/net/disk_cache/histogram_macros.h
+++ b/net/disk_cache/histogram_macros.h
@@ -25,13 +25,8 @@
 #define UMA_HISTOGRAM_AGE_MS(name, initial_time)\
     UMA_HISTOGRAM_TIMES(name, Time::Now() - initial_time)
 
-#define UMA_HISTOGRAM_CACHE_ERROR(name, sample) do { \
-    static scoped_refptr<Histogram> counter = \
-        LinearHistogram::LinearHistogramFactoryGet(\
-            (name), 1, 49, 50); \
-    counter->SetFlags(kUmaTargetedHistogramFlag); \
-    counter->Add(sample); \
-  } while (0)
+#define UMA_HISTOGRAM_CACHE_ERROR(name, sample) \
+    UMA_HISTOGRAM_ENUMERATION(name, sample, 50)
 
 #ifdef NET_DISK_CACHE_BACKEND_IMPL_CC_
 #define BACKEND_OBJ this
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 44f1aa1..ec87b8c 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -1146,18 +1146,14 @@ int FtpNetworkTransaction::DoDataReadComplete(int result) {
   return result;
 }
 
-// We're using a histogram as a group of counters.  We're only interested in
-// the values of the counters.  Ignore the shape, average, and standard
-// deviation of the histograms because they are meaningless.
+// We're using a histogram as a group of counters, with one bucket for each
+// enumeration value.  We're only interested in the values of the counters.
+// Ignore the shape, average, and standard deviation of the histograms because
+// they are meaningless.
 //
-// We use two groups of counters.  In the first group (counter1), each counter
-// is a boolean (0 or 1) that indicates whether the user has seen an error
-// of that type during that session.  In the second group (counter2), each
-// counter is the number of errors of that type the user has seen during
-// that session.
-//
-// Each histogram has an unused bucket at the end to allow seamless future
-// expansion.
+// We use two histograms.  In the first histogram we tally whether the user has
+// seen an error of that type during the session.  In the second histogram we
+// tally the total number of times the users sees each errer.
 void FtpNetworkTransaction::RecordDataConnectionError(int result) {
   // Gather data for http://crbug.com/3073. See how many users have trouble
   // establishing FTP data connection in passive FTP mode.
@@ -1205,23 +1201,15 @@ void FtpNetworkTransaction::RecordDataConnectionError(int result) {
       break;
   };
   static bool had_error_type[NUM_OF_NET_ERROR_TYPES];
-  static scoped_refptr<Histogram> error_flagged =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.FtpDataConnectionErrorHappened",
-          1, NUM_OF_NET_ERROR_TYPES, NUM_OF_NET_ERROR_TYPES + 1);
-  static scoped_refptr<Histogram> error_counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.FtpDataConnectionErrorCount",
-          1, NUM_OF_NET_ERROR_TYPES, NUM_OF_NET_ERROR_TYPES + 1);
 
   DCHECK(type >= 0 && type < NUM_OF_NET_ERROR_TYPES);
   if (!had_error_type[type]) {
     had_error_type[type] = true;
-    error_flagged->SetFlags(kUmaTargetedHistogramFlag);
-    error_flagged->Add(type);
+    UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorHappened",
+        type, NUM_OF_NET_ERROR_TYPES);
   }
-  error_counter->SetFlags(kUmaTargetedHistogramFlag);
-  error_counter->Add(type);
+  UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorCount",
+      type, NUM_OF_NET_ERROR_TYPES);
 }
 
 }  // namespace net
diff --git a/net/ftp/ftp_server_type_histograms.cc b/net/ftp/ftp_server_type_histograms.cc
index 60ab9b9..de90d72 100644
--- a/net/ftp/ftp_server_type_histograms.cc
+++ b/net/ftp/ftp_server_type_histograms.cc
@@ -8,38 +8,26 @@
 
 namespace net {
 
-// We're using a histogram as a group of counters.  We're only interested in
-// the values of the counters.  Ignore the shape, average, and standard
-// deviation of the histograms because they are meaningless.
+// We're using a histogram as a group of counters, with one bucket for each
+// enumeration value.  We're only interested in the values of the counters.
+// Ignore the shape, average, and standard deviation of the histograms because
+// they are meaningless.
 //
-// We use two groups of counters.  In the first group (counter1), each counter
-// is a boolean (0 or 1) that indicates whether the user has seen an FTP server
-// of that type during that session.  In the second group (counter2), each
-// counter is the number of transactions with FTP server of that type the user
-// has made during that session.
-//
-// Each histogram has an unused bucket at the end to allow seamless future
-// expansion.
+// We use two histograms.  In the first histogram we tally whether the user has
+// seen an FTP server of a given type during that session.  In the second
+// histogram we tally the number of transactions with FTP server of a given type
+// the user has made during that session.
 void UpdateFtpServerTypeHistograms(FtpServerType type) {
   static bool had_server_type[NUM_OF_SERVER_TYPES];
-  static scoped_refptr<Histogram> counter1 =
-      LinearHistogram::LinearHistogramFactoryGet("Net.HadFtpServerType",
-                                                 1, NUM_OF_SERVER_TYPES,
-                                                 NUM_OF_SERVER_TYPES + 1);
-  static scoped_refptr<Histogram> counter2 =
-      LinearHistogram::LinearHistogramFactoryGet("Net.FtpServerTypeCount",
-                                                 1, NUM_OF_SERVER_TYPES,
-                                                 NUM_OF_SERVER_TYPES + 1);
-
   if (type >= 0 && type < NUM_OF_SERVER_TYPES) {
     if (!had_server_type[type]) {
       had_server_type[type] = true;
-      counter1->SetFlags(kUmaTargetedHistogramFlag);
-      counter1->Add(type);
+      UMA_HISTOGRAM_ENUMERATION("Net.HadFtpServerType",
+                                type, NUM_OF_SERVER_TYPES);
     }
   }
-  counter2->SetFlags(kUmaTargetedHistogramFlag);
-  counter2->Add(type);
+  UMA_HISTOGRAM_ENUMERATION("Net.FtpServerTypeCount",
+                            type, NUM_OF_SERVER_TYPES);
 }
 
 }  // namespace net
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index eb50716..d61188a 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1157,22 +1157,13 @@ void HttpNetworkTransaction::LogTCPConnectedMetrics(
         100);
   }
 
-  static scoped_refptr<Histogram> tcp_socket_type_counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.TCPSocketType",
-          1, ClientSocketHandle::NUM_TYPES, ClientSocketHandle::NUM_TYPES + 1);
-  tcp_socket_type_counter->SetFlags(kUmaTargetedHistogramFlag);
-  tcp_socket_type_counter->Add(handle.reuse_type());
+  UMA_HISTOGRAM_ENUMERATION("Net.TCPSocketType", handle.reuse_type(),
+      ClientSocketHandle::NUM_TYPES);
 
   if (use_late_binding_histogram) {
-    static scoped_refptr<Histogram> tcp_socket_type_counter2 =
-        LinearHistogram::LinearHistogramFactoryGet(
-            FieldTrial::MakeName("Net.TCPSocketType",
-                                 "SocketLateBinding").data(),
-            1, ClientSocketHandle::NUM_TYPES,
-            ClientSocketHandle::NUM_TYPES + 1);
-    tcp_socket_type_counter2->SetFlags(kUmaTargetedHistogramFlag);
-    tcp_socket_type_counter2->Add(handle.reuse_type());
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("Net.TCPSocketType", "SocketLateBinding"),
+        handle.reuse_type(), ClientSocketHandle::NUM_TYPES);
   }
 
   UMA_HISTOGRAM_CLIPPED_TIMES(
@@ -1230,22 +1221,14 @@ void HttpNetworkTransaction::LogIOErrorMetrics(
   static const bool use_late_binding_histogram =
       !FieldTrial::MakeName("", "SocketLateBinding").empty();
 
-  static scoped_refptr<Histogram> io_error_socket_type_counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.IOError_SocketReuseType",
-          1, ClientSocketHandle::NUM_TYPES, ClientSocketHandle::NUM_TYPES + 1);
-  io_error_socket_type_counter->SetFlags(kUmaTargetedHistogramFlag);
-  io_error_socket_type_counter->Add(handle.reuse_type());
+  UMA_HISTOGRAM_ENUMERATION("Net.IOError_SocketReuseType",
+       handle.reuse_type(), ClientSocketHandle::NUM_TYPES);
 
   if (use_late_binding_histogram) {
-    static scoped_refptr<Histogram> io_error_socket_type_counter =
-        LinearHistogram::LinearHistogramFactoryGet(
-            FieldTrial::MakeName("Net.IOError_SocketReuseType",
-                                 "SocketLateBinding").data(),
-            1, ClientSocketHandle::NUM_TYPES,
-            ClientSocketHandle::NUM_TYPES + 1);
-    io_error_socket_type_counter->SetFlags(kUmaTargetedHistogramFlag);
-    io_error_socket_type_counter->Add(handle.reuse_type());
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("Net.IOError_SocketReuseType",
+                             "SocketLateBinding"),
+        handle.reuse_type(), ClientSocketHandle::NUM_TYPES);
   }
 
   switch (handle.reuse_type()) {
diff --git a/net/socket_stream/socket_stream_metrics.cc b/net/socket_stream/socket_stream_metrics.cc
index 6220eb5..625a491 100644
--- a/net/socket_stream/socket_stream_metrics.cc
+++ b/net/socket_stream/socket_stream_metrics.cc
@@ -83,21 +83,13 @@ void SocketStreamMetrics::OnClose() {
 }
 
 void SocketStreamMetrics::CountProtocolType(ProtocolType protocol_type) {
-  static scoped_refptr<Histogram> counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.SocketStream.ProtocolType",
-          0, NUM_PROTOCOL_TYPES, NUM_PROTOCOL_TYPES + 1);
-  counter->SetFlags(kUmaTargetedHistogramFlag);
-  counter->Add(protocol_type);
+  UMA_HISTOGRAM_ENUMERATION("Net.SocketStream.ProtocolType",
+       protocol_type, NUM_PROTOCOL_TYPES);
 }
 
 void SocketStreamMetrics::CountConnectionType(ConnectionType connection_type) {
-  static scoped_refptr<Histogram> counter =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Net.SocketStream.ConnectionType",
-          1, NUM_CONNECTION_TYPES, NUM_CONNECTION_TYPES + 1);
-  counter->SetFlags(kUmaTargetedHistogramFlag);
-  counter->Add(connection_type);
+  UMA_HISTOGRAM_ENUMERATION("Net.SocketStream.ConnectionType",
+       connection_type, NUM_CONNECTION_TYPES);
 }
 
 
diff --git a/net/socket_stream/socket_stream_metrics_unittest.cc b/net/socket_stream/socket_stream_metrics_unittest.cc
index e29ee3f..7d44f79 100644
--- a/net/socket_stream/socket_stream_metrics_unittest.cc
+++ b/net/socket_stream/socket_stream_metrics_unittest.cc
@@ -42,7 +42,7 @@ TEST(SocketStreamMetricsTest, ProtocolType) {
 
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ProtocolType", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
   Histogram::SampleSet sample;
   histogram->SnapshotSample(&sample);
@@ -75,7 +75,7 @@ TEST(SocketStreamMetricsTest, ConnectionType) {
 
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ConnectionType", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
   Histogram::SampleSet sample;
   histogram->SnapshotSample(&sample);
@@ -134,46 +134,46 @@ TEST(SocketStreamMetricsTest, OtherNumbers) {
   // ConnectionLatency.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ConnectionLatency", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // ConnectionEstablish.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ConnectionEstablish", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // Duration.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.Duration", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // ReceivedBytes.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ReceivedBytes", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(11, sample.sum() - original_received_bytes);  // 11 bytes read.
 
   // ReceivedCounts.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.ReceivedCounts", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(2, sample.sum() - original_received_counts);  // 2 read requests.
 
   // SentBytes.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.SentBytes", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(222, sample.sum() - original_sent_bytes);  // 222 bytes sent.
 
   // SentCounts.
   ASSERT_TRUE(StatisticsRecorder::FindHistogram(
       "Net.SocketStream.SentCounts", &histogram));
-  EXPECT_EQ(kUmaTargetedHistogramFlag, histogram->flags());
+  EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(3, sample.sum() - original_sent_counts);  // 3 write requests.
 }