[Most Visited] Log suggestion provider to UMA

chrome/browser/search/most_visited_iframe_source.h

 // Copyright 2013 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.
 
 #ifndef CHROME_BROWSER_SEARCH_MOST_VISITED_IFRAME_SOURCE_H_
 #define CHROME_BROWSER_SEARCH_MOST_VISITED_IFRAME_SOURCE_H_
 
 #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 {
  public:
   MostVisitedIframeSource();
   virtual ~MostVisitedIframeSource();
 
   // Number of Most Visited elements on the NTP for logging purposes.
   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(
       const std::string& path_and_query,
       int render_process_id,
       int render_view_id,
       const content::URLDataSource::GotDataCallback& callback) OVERRIDE;
 
  protected:
   // Overridden from IframeSource:
   virtual std::string GetSource() const OVERRIDE;
 
   virtual bool ServesPath(const std::string& path) const OVERRIDE;
 
   DISALLOW_COPY_AND_ASSIGN(MostVisitedIframeSource);
 };
 
 #endif  // CHROME_BROWSER_SEARCH_MOST_VISITED_IFRAME_SOURCE_H_