PrintPreview: Added new histograms for print preview.

chrome/browser/ui/webui/print_preview_handler.cc

Index: chrome/browser/ui/webui/print_preview_handler.cc
diff --git a/chrome/browser/ui/webui/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview_handler.cc
index 4a8e26d2da53b848c94d1668cb54b4c22c97f2ff..963cc5553d838be3c54e1edf89df2d87d62f531d 100644
--- a/chrome/browser/ui/webui/print_preview_handler.cc
+++ b/chrome/browser/ui/webui/print_preview_handler.cc
@@ -8,6 +8,7 @@
 
 #include "base/i18n/file_util_icu.h"
 #include "base/json/json_reader.h"
+#include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
@@ -99,6 +100,11 @@ class PrintSystemTaskProxy
     printing::PrinterList printer_list;
     print_backend_->EnumeratePrinters(&printer_list);
     int i = 0;

[Lei Zhang, 2011/05/13 22:56:20]

This is part of the for loop below. Don't separate it from the for loop.

[kmadhusu, 2011/05/13 23:37:12]

Done.

+
+    // Record the total number of printers.
+    UMA_HISTOGRAM_COUNTS("PrintPreview.TotalNumberOfPrinters",

[Lei Zhang, 2011/05/13 22:56:20]

Can you just name this "NumberOfPrinters" ?

[Lei Zhang, 2011/05/13 22:56:20]

jar: does this record "a user has N printers"? Or will it increment the counter
by N every time - thus if this gets called M times, it'll report the user has M
x N printers.

[kmadhusu, 2011/05/13 23:37:12]

Done.

+                         printer_list.size());
+
     for (printing::PrinterList::iterator index = printer_list.begin();
          index != printer_list.end(); ++index, ++i) {
       DictionaryValue* printer_info = new DictionaryValue;
@@ -236,7 +242,10 @@ class PrintToPdfTask : public Task {
 FilePath* PrintPreviewHandler::last_saved_path_ = NULL;
 
 PrintPreviewHandler::PrintPreviewHandler()
-    : print_backend_(printing::PrintBackend::CreateInstance(NULL)) {
+    : print_backend_(printing::PrintBackend::CreateInstance(NULL)),
+      regenerate_preview_request_count_(0),
+      manage_printers_dialog_request_count_(0),
+      print_preview_failed_count_(0) {
 }
 
 PrintPreviewHandler::~PrintPreviewHandler() {
@@ -275,8 +284,12 @@ void PrintPreviewHandler::HandleGetPrinters(const ListValue*) {
 }
 
 void PrintPreviewHandler::HandleGetPreview(const ListValue* args) {
+  // Increment request count.
+  ++regenerate_preview_request_count_;
+
   TabContents* initiator_tab = GetInitiatorTab();
   if (!initiator_tab) {
+    print_preview_failed_count_++;
     web_ui_->CallJavascriptFunction("printPreviewFailed");
     return;
   }
@@ -289,6 +302,15 @@ void PrintPreviewHandler::HandleGetPreview(const ListValue* args) {
 }
 
 void PrintPreviewHandler::HandlePrint(const ListValue* args) {
+  ReportStats();
+  UMA_HISTOGRAM_COUNTS("PrintPreview.Print", 1);
+
+  // Record the number of times the user requests to regenerate preview data
+  // before printing.
+  UMA_HISTOGRAM_COUNTS(
+      "PrintPreview.RegeneratePreviewRequestCount.BeforePrint",

[Lei Zhang, 2011/05/13 22:56:20]

I don't think we need Count in the name.

[kmadhusu, 2011/05/13 23:37:12]

Done.

+      regenerate_preview_request_count_);
+
   TabContents* initiator_tab = GetInitiatorTab();
   if (initiator_tab) {
     RenderViewHost* rvh = initiator_tab->render_view_host();
@@ -306,6 +328,8 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
       TabContentsWrapper::GetCurrentWrapperForContents(preview_tab());
 
   if (print_to_pdf) {
+    UMA_HISTOGRAM_COUNTS("PrintPreview.PrintToPDF", 1);
+
     // Pre-populating select file dialog with print job title.
     string16 print_job_title_utf16 =
         preview_tab_wrapper->print_view_manager()->RenderSourceName();
@@ -352,6 +376,8 @@ void PrintPreviewHandler::HandleGetPrinterCapabilities(
 }
 
 void PrintPreviewHandler::HandleShowSystemDialog(const ListValue* args) {
+  ReportStats();
+  UMA_HISTOGRAM_COUNTS("PrintPreview.ShowAdvanceSettings", 1);
   TabContents* initiator_tab = GetInitiatorTab();
   if (!initiator_tab)
     return;
@@ -365,13 +391,35 @@ void PrintPreviewHandler::HandleShowSystemDialog(const ListValue* args) {
 }
 
 void PrintPreviewHandler::HandleManagePrinters(const ListValue* args) {
+  manage_printers_dialog_request_count_++;

[Lei Zhang, 2011/05/13 22:56:20]

nit: can we be consistent about ++i vs i++?

[kmadhusu, 2011/05/13 23:37:12]

Done.

   printing::PrinterManagerDialog::ShowPrinterManagerDialog();
 }
 
 void PrintPreviewHandler::HandleClosePreviewTab(const ListValue* args) {
+  ReportStats();
+  UMA_HISTOGRAM_COUNTS("PrintPreview.Cancel", 1);
+
+  // Record the number of times the user requests to regenerate preview data
+  // before cancelling.
+  UMA_HISTOGRAM_COUNTS(

[Lei Zhang, 2011/05/13 22:56:20]

Do you need to check and see if the count is greater than 0 here and in other
spots? If so, add the if statement, otherwise remove it from ReportStats().

[kmadhusu, 2011/05/13 23:37:12]

(repeating our in-person conversation) I have added the if statement only in the
places where we don't want to track the '0' values.

+      "PrintPreview.RegeneratePreviewRequestCount.BeforeCancel",
+      regenerate_preview_request_count_);
+
   ActivateInitiatorTabAndClosePreviewTab();
 }
 
+void PrintPreviewHandler::ReportStats() {
+  if (print_preview_failed_count_ > 0) {
+    UMA_HISTOGRAM_COUNTS("PrintPreview.Failed.InitiatorTabDoesNotExist",
+                         print_preview_failed_count_);
+  }
+
+  if (manage_printers_dialog_request_count_ > 0) {
+    UMA_HISTOGRAM_COUNTS("PrintPreview.ManagePrinters",
+                         manage_printers_dialog_request_count_);
+  }
+}
+
 void PrintPreviewHandler::ActivateInitiatorTabAndClosePreviewTab() {
   TabContents* initiator_tab = GetInitiatorTab();
   if (initiator_tab)