[Most Visited] Log suggestion provider to UMA

chrome/browser/search/most_visited_iframe_source.h

Index: chrome/browser/search/most_visited_iframe_source.h
diff --git a/chrome/browser/search/most_visited_iframe_source.h b/chrome/browser/search/most_visited_iframe_source.h
index 7344cc82724f920054e914ea0979733c0f9cd663..4ad63804774c4325e1cf66023200d0128c3c5d58 100644
--- a/chrome/browser/search/most_visited_iframe_source.h
+++ b/chrome/browser/search/most_visited_iframe_source.h
@@ -7,8 +7,24 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/search/iframe_source.h"
 
+class MostVisitedIframeSource;

[beaudoin, 2013/12/12 19:37:21]

Do you really need to forward declare this for the macro? I would expect not.

[Mathieu, 2013/12/12 21:39:13]

Done.

+
+#define MV_CLICK_HISTOGRAM(position, provider)                              \

[beaudoin, 2013/12/12 19:37:21]

Do you have to do this in the .h? It pollutes the global namespace.

[Mathieu, 2013/12/12 21:39:13]

Done.

+  do {                                                                      \
+    std::string histogram_name = provider ?                                 \
+        base::StringPrintf(                                                 \
+            MostVisitedIframeSource::kMostVisitedHistogramWithProvider,     \
+            provider) : MostVisitedIframeSource::kMostVisitedHistogramName; \
+    base::HistogramBase* counter = base::LinearHistogram::FactoryGet(       \
+        histogram_name, 1, MostVisitedIframeSource::kNumMostVisited,        \
+        MostVisitedIframeSource::kNumMostVisited + 1,                       \
+        base::Histogram::kUmaTargetedHistogramFlag);                        \

[beaudoin, 2013/12/12 19:39:51]

Missing the includes for all the histogram stuff.

[Mathieu, 2013/12/12 21:39:13]

Done.

+    counter->Add(position);                                                 \
+  } while (0)
+
 // Serves HTML for displaying suggestions using iframes, e.g.
 // chrome-search://suggestion/loader.html
 class MostVisitedIframeSource : public IframeSource {
@@ -20,6 +36,8 @@ class MostVisitedIframeSource : public IframeSource {
   static const int kNumMostVisited;
   // Name of the histogram keeping track of Most Visited clicks.
   static const char kMostVisitedHistogramName[];
+  // Format string keeping track of MV clicks specific to a provider.

[beaudoin, 2013/12/12 19:43:06]

Format string to generate the name of the histogram...

[Mathieu, 2013/12/12 21:39:13]

Done.

+  static const char kMostVisitedHistogramWithProvider[];
 
   // Overridden from IframeSource. Public for testing.
   virtual void StartDataRequest(