Improve print preview UMAs based on jar's feedback.

BUG=80917
TEST=NONE

Review URL: http://codereview.chromium.org/7082030

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

6 files changed, 85 insertions, 109 deletions

diff --git a/chrome/browser/ui/webui/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview_handler.cc
index c2912bc..ff8c16c2 100644
--- a/chrome/browser/ui/webui/print_preview_handler.cc
+++ b/chrome/browser/ui/webui/print_preview_handler.cc
@@ -55,18 +55,6 @@ const char kColorDevice[] = "ColorDevice";
 const char kPskColor[] = "psk:Color";
 #endif
 
-// UMA histogram names and buckets.
-const char kUserAction[] = "PrintPreview.UserAction";
-const char kManagePrinters[] = "PrintPreview.ManagePrinters";
-const char kNumberOfPrinters[] = "PrintPreview.NumberOfPrinters";
-const char kPrintSettings[] = "PrintPreview.PrintSettings";
-const char kRegeneratePreviewRequestsRcvdBeforeCancel[] =
-    "PrintPreview.RegeneratePreviewRequest.BeforeCancel";
-const char kRegeneratePreviewRequestsRcvdBeforePrint[] =
-    "PrintPreview.RegeneratePreviewRequest.BeforePrint";
-const char kPagesPrintedToPDF[] = "PrintPreview.PageCount.PrintToPDF";
-const char kPagesPrintedToPrinter[] = "PrintPreview.PageCount.PrintToPrinter";
-
 enum UserActionBuckets {
   PRINT_TO_PRINTER,
   PRINT_TO_PDF,
@@ -88,6 +76,15 @@ enum PrintSettingsBuckets {
   PRINT_SETTINGS_BUCKET_BOUNDARY
 };
 
+void ReportUserActionHistogram(enum UserActionBuckets event) {
+  UMA_HISTOGRAM_ENUMERATION("PrintPreview.UserAction", event,
+                            USERACTION_BUCKET_BOUNDARY);
+}
+
+void ReportPrintSettingHistogram(enum PrintSettingsBuckets setting) {
+  UMA_HISTOGRAM_ENUMERATION("PrintPreview.PrintSettings", setting,
+                            PRINT_SETTINGS_BUCKET_BOUNDARY);
+}
 
 // Get the print job settings dictionary from |args|. The caller takes
 // ownership of the returned DictionaryValue. Returns NULL on failure.
@@ -139,34 +136,20 @@ int GetPageCountFromSettingsDictionary(const DictionaryValue& settings) {
 // Track the popularity of print settings and report the stats.
 void ReportPrintSettingsStats(const DictionaryValue& settings) {
   bool landscape;
-  if (settings.GetBoolean(printing::kSettingLandscape, &landscape)) {
-    UMA_HISTOGRAM_ENUMERATION(kPrintSettings,
-                              landscape ? LANDSCAPE : PORTRAIT,
-                              PRINT_SETTINGS_BUCKET_BOUNDARY);
-  }
+  if (settings.GetBoolean(printing::kSettingLandscape, &landscape))
+    ReportPrintSettingHistogram(landscape ? LANDSCAPE : PORTRAIT);
 
   bool collate;
-  if (settings.GetBoolean(printing::kSettingCollate, &collate)) {
-    if (collate) {
-      UMA_HISTOGRAM_ENUMERATION(kPrintSettings,
-                                COLLATE,
-                                PRINT_SETTINGS_BUCKET_BOUNDARY);
-    }
-  }
+  if (settings.GetBoolean(printing::kSettingCollate, &collate) && collate)
+    ReportPrintSettingHistogram(COLLATE);
 
   int duplex_mode;
-  if (settings.GetInteger(printing::kSettingDuplexMode, &duplex_mode)) {
-    UMA_HISTOGRAM_ENUMERATION(kPrintSettings,
-                              duplex_mode ? DUPLEX : SIMPLEX,
-                              PRINT_SETTINGS_BUCKET_BOUNDARY);
-  }
+  if (settings.GetInteger(printing::kSettingDuplexMode, &duplex_mode))
+    ReportPrintSettingHistogram(duplex_mode ? DUPLEX : SIMPLEX);
 
   bool is_color;
-  if (settings.GetBoolean(printing::kSettingColor, &is_color)) {
-    UMA_HISTOGRAM_ENUMERATION(kPrintSettings,
-                              is_color ? COLOR : BLACK_AND_WHITE,
-                              PRINT_SETTINGS_BUCKET_BOUNDARY);
-  }
+  if (settings.GetBoolean(printing::kSettingColor, &is_color))
+    ReportPrintSettingHistogram(is_color ? COLOR : BLACK_AND_WHITE);
 }
 
 }  // namespace
@@ -212,7 +195,8 @@ class PrintSystemTaskProxy
 
     if (!has_logged_printers_count_) {
       // Record the total number of printers.
-      UMA_HISTOGRAM_COUNTS(kNumberOfPrinters, printer_list.size());
+      UMA_HISTOGRAM_COUNTS("PrintPreview.NumberOfPrinters",
+                           printer_list.size());
     }
 
     int i = 0;
@@ -361,9 +345,7 @@ PrintPreviewHandler::PrintPreviewHandler()
       manage_printers_dialog_request_count_(0),
       reported_failed_preview_(false),
       has_logged_printers_count_(false) {
-  UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                            PREVIEW_STARTED,
-                            USERACTION_BUCKET_BOUNDARY);
+  ReportUserActionHistogram(PREVIEW_STARTED);
 }
 
 PrintPreviewHandler::~PrintPreviewHandler() {
@@ -425,9 +407,7 @@ void PrintPreviewHandler::HandleGetPreview(const ListValue* args) {
   TabContents* initiator_tab = GetInitiatorTab();
   if (!initiator_tab) {
     if (!reported_failed_preview_) {
-      UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                                PREVIEW_FAILED,
-                                USERACTION_BUCKET_BOUNDARY);
+      ReportUserActionHistogram(PREVIEW_FAILED);
       reported_failed_preview_ = true;
     }
     web_ui_->CallJavascriptFunction("printPreviewFailed");
@@ -447,7 +427,7 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
 
   // Record the number of times the user requests to regenerate preview data
   // before printing.
-  UMA_HISTOGRAM_COUNTS(kRegeneratePreviewRequestsRcvdBeforePrint,
+  UMA_HISTOGRAM_COUNTS("PrintPreview.RegeneratePreviewRequest.BeforePrint",
                        regenerate_preview_request_count_);
 
   TabContents* initiator_tab = GetInitiatorTab();
@@ -467,10 +447,8 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
       TabContentsWrapper::GetCurrentWrapperForContents(preview_tab());
 
   if (print_to_pdf) {
-    UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                              PRINT_TO_PDF,
-                              USERACTION_BUCKET_BOUNDARY);
-    UMA_HISTOGRAM_COUNTS(kPagesPrintedToPDF,
+    ReportUserActionHistogram(PRINT_TO_PDF);
+    UMA_HISTOGRAM_COUNTS("PrintPreview.PageCount.PrintToPDF",
                          GetPageCountFromSettingsDictionary(*settings));
 
     // Pre-populating select file dialog with print job title.
@@ -491,10 +469,8 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
     SelectFile(default_filename);
   } else {
     ReportPrintSettingsStats(*settings);
-    UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                              PRINT_TO_PRINTER,
-                              USERACTION_BUCKET_BOUNDARY);
-    UMA_HISTOGRAM_COUNTS(kPagesPrintedToPrinter,
+    ReportUserActionHistogram(PRINT_TO_PRINTER);
+    UMA_HISTOGRAM_COUNTS("PrintPreview.PageCount.PrintToPrinter",
                          GetPageCountFromSettingsDictionary(*settings));
     g_browser_process->background_printing_manager()->OwnTabContents(
         preview_tab_wrapper);
@@ -528,9 +504,7 @@ void PrintPreviewHandler::HandleGetPrinterCapabilities(
 
 void PrintPreviewHandler::HandleShowSystemDialog(const ListValue* args) {
   ReportStats();
-  UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                            FALLBACK_TO_ADVANCED_SETTINGS_DIALOG,
-                            USERACTION_BUCKET_BOUNDARY);
+  ReportUserActionHistogram(FALLBACK_TO_ADVANCED_SETTINGS_DIALOG);
 
   TabContents* initiator_tab = GetInitiatorTab();
   if (!initiator_tab)
@@ -551,20 +525,18 @@ void PrintPreviewHandler::HandleManagePrinters(const ListValue* args) {
 
 void PrintPreviewHandler::HandleClosePreviewTab(const ListValue* args) {
   ReportStats();
-  UMA_HISTOGRAM_ENUMERATION(kUserAction,
-                            CANCEL,
-                            USERACTION_BUCKET_BOUNDARY);
+  ReportUserActionHistogram(CANCEL);
 
   // Record the number of times the user requests to regenerate preview data
   // before cancelling.
-  UMA_HISTOGRAM_COUNTS(kRegeneratePreviewRequestsRcvdBeforeCancel,
+  UMA_HISTOGRAM_COUNTS("PrintPreview.RegeneratePreviewRequest.BeforeCancel",
                        regenerate_preview_request_count_);
 
   ActivateInitiatorTabAndClosePreviewTab();
 }
 
 void PrintPreviewHandler::ReportStats() {
-  UMA_HISTOGRAM_COUNTS(kManagePrinters,
+  UMA_HISTOGRAM_COUNTS("PrintPreview.ManagePrinters",
                        manage_printers_dialog_request_count_);
 }
 
diff --git a/chrome/renderer/print_web_view_helper.cc b/chrome/renderer/print_web_view_helper.cc
index 3080c16..ef8e5d5 100644
--- a/chrome/renderer/print_web_view_helper.cc
+++ b/chrome/renderer/print_web_view_helper.cc
@@ -661,38 +661,32 @@ bool PrintWebViewHelper::RenderPagesForPreview(WebKit::WebFrame* frame,
   return CreatePreviewDocument(print_settings, frame, node);
 }
 
-void PrintWebViewHelper::ReportPreviewGenerationTime(base::TimeDelta time,
-                                                     int pages) {
-  if (pages <= 0) {
+base::TimeTicks PrintWebViewHelper::ReportPreviewPageRenderTime(
+    base::TimeTicks start_time) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  UMA_HISTOGRAM_TIMES("PrintPreview.RenderPDFPageTime", now - start_time);
+  return now;
+}
+
+void PrintWebViewHelper::ReportTotalPreviewGenerationTime(
+        int selected_pages_length, int total_pages,
+        base::TimeDelta render_time, base::TimeDelta total_time) {
+
+  if (selected_pages_length == 0)
+    selected_pages_length = total_pages;
+
+  if (selected_pages_length <= 0) {
     // This shouldn't happen, but this makes sure it doesn't affect the
     // statistics if it does.
     return;
   }
 
-  UMA_HISTOGRAM_TIMES("PrintPreview.RenderAndGeneratePDFTime", time);
-
-  if (pages == 1) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.1Page", time);
-  } else if (pages == 2) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.2Pages", time);
-  } else if (pages <= 10) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.3-10Pages", time);
-  } else if (pages <= 25) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.11-25Pages", time);
-  } else if (pages <= 50) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.26-50Pages", time);
-  } else if (pages <= 100) {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.51-100Pages", time);
-  } else {
-    UMA_HISTOGRAM_TIMES(
-        "PrintPreview.RenderAndGeneratePDFTime.101+Pages", time);
-  }
+  UMA_HISTOGRAM_MEDIUM_TIMES("PrintPreview.RenderToPDFTime",
+                             render_time);
+  UMA_HISTOGRAM_MEDIUM_TIMES("PrintPreview.RenderAndGeneratePDFTime",
+                             total_time);
+  UMA_HISTOGRAM_MEDIUM_TIMES("PrintPreview.RenderAndGeneratePDFTimeAvgPerPage",
+                             total_time / selected_pages_length);
 }
 
 #if defined(OS_POSIX)
diff --git a/chrome/renderer/print_web_view_helper.h b/chrome/renderer/print_web_view_helper.h
index db2e280..ed9c8e2 100644
--- a/chrome/renderer/print_web_view_helper.h
+++ b/chrome/renderer/print_web_view_helper.h
@@ -230,7 +230,13 @@ class PrintWebViewHelper : public RenderViewObserver ,
 
   bool GetPrintFrame(WebKit::WebFrame** frame);
 
-  void ReportPreviewGenerationTime(base::TimeDelta time, int pages);
+  // This reports the current time - |start_time| as the time to render
+  // a page and returns the current time.
+  base::TimeTicks ReportPreviewPageRenderTime(base::TimeTicks start_time);
+  void ReportTotalPreviewGenerationTime(int selected_pages_length,
+                                        int total_pages,
+                                        base::TimeDelta render_time,
+                                        base::TimeDelta total_time);
 
   // Script Initiated Printing ------------------------------------------------
 
diff --git a/chrome/renderer/print_web_view_helper_linux.cc b/chrome/renderer/print_web_view_helper_linux.cc
index 6793e4c..08042b9 100644
--- a/chrome/renderer/print_web_view_helper_linux.cc
+++ b/chrome/renderer/print_web_view_helper_linux.cc
@@ -166,6 +166,7 @@ bool PrintWebViewHelper::RenderPages(const PrintMsg_PrintPages_Params& params,
     return false;
 
   base::TimeTicks begin_time = base::TimeTicks::Now();
+  base::TimeTicks page_begin_time = begin_time;
 
   PrintMsg_PrintPage_Params page_params;
   page_params.params = printParams;
@@ -174,27 +175,29 @@ bool PrintWebViewHelper::RenderPages(const PrintMsg_PrintPages_Params& params,
     for (int i = 0; i < *page_count; ++i) {
       page_params.page_number = i;
       PrintPageInternal(page_params, canvas_size, frame, metafile);
+      if (is_preview) {
+        page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
+      }
     }
   } else {
     for (size_t i = 0; i < params.pages.size(); ++i) {
       page_params.page_number = params.pages[i];
       PrintPageInternal(page_params, canvas_size, frame, metafile);
+      if (is_preview) {
+        page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
+      }
     }
   }
 
-  if (is_preview) {
-    UMA_HISTOGRAM_TIMES("PrintPreview.RenderToPDFTime",
-                        base::TimeTicks::Now() - begin_time);
-  }
+  base::TimeDelta render_time = base::TimeTicks::Now() - begin_time;
 
+  prep_frame_view.FinishPrinting();
   metafile->FinishDocument();
 
   if (is_preview) {
-    int preview_page_count = params.pages.size();
-    if (!preview_page_count)
-      preview_page_count = *page_count;
-    ReportPreviewGenerationTime(base::TimeTicks::Now() - begin_time,
-                                preview_page_count);
+    ReportTotalPreviewGenerationTime(params.pages.size(), *page_count,
+                                     render_time,
+                                     base::TimeTicks::Now() - begin_time);
   }
   return true;
 }
diff --git a/chrome/renderer/print_web_view_helper_mac.mm b/chrome/renderer/print_web_view_helper_mac.mm
index 1f1dedb..e46320e 100644
--- a/chrome/renderer/print_web_view_helper_mac.mm
+++ b/chrome/renderer/print_web_view_helper_mac.mm
@@ -76,11 +76,13 @@ bool PrintWebViewHelper::CreatePreviewDocument(
                          printParams.printable_size.height());
 
   base::TimeTicks begin_time = base::TimeTicks::Now();
+  base::TimeTicks page_begin_time = begin_time;
 
   if (params.pages.empty()) {
     for (int i = 0; i < page_count; ++i) {
       RenderPage(printParams.page_size, content_area, scale_factor, i, frame,
                  &metafile);
+      page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
     }
   } else {
     for (size_t i = 0; i < params.pages.size(); ++i) {
@@ -88,19 +90,18 @@ bool PrintWebViewHelper::CreatePreviewDocument(
         break;
       RenderPage(printParams.page_size, content_area, scale_factor,
                  static_cast<int>(params.pages[i]), frame, &metafile);
+      page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
     }
   }
 
-  UMA_HISTOGRAM_TIMES("PrintPreview.RenderToPDFTime",
-                      base::TimeTicks::Now() - begin_time);
+  base::TimeDelta render_time = base::TimeTicks::Now() - begin_time;
 
+  prep_frame_view.FinishPrinting();
   metafile.FinishDocument();
 
-  int preview_page_count = params.pages.size();
-  if (!preview_page_count)
-    preview_page_count = page_count;
-  ReportPreviewGenerationTime(base::TimeTicks::Now() - begin_time,
-                              preview_page_count);
+  ReportTotalPreviewGenerationTime(params.pages.size(), page_count,
+                                   render_time,
+                                   base::TimeTicks::Now() - begin_time);
 
   PrintHostMsg_DidPreviewDocument_Params preview_params;
   preview_params.data_size = metafile.GetDataSize();
diff --git a/chrome/renderer/print_web_view_helper_win.cc b/chrome/renderer/print_web_view_helper_win.cc
index 3849788..8f55288 100644
--- a/chrome/renderer/print_web_view_helper_win.cc
+++ b/chrome/renderer/print_web_view_helper_win.cc
@@ -143,11 +143,13 @@ bool PrintWebViewHelper::CreatePreviewDocument(
                                     print_params.dpi);
 
   base::TimeTicks begin_time = base::TimeTicks::Now();
+  base::TimeTicks page_begin_time = begin_time;
 
   if (params.pages.empty()) {
     for (int i = 0; i < page_count; ++i) {
       float scale_factor = shrink;
       RenderPage(print_params, &scale_factor, i, true, frame, &metafile);
+      page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
     }
   } else {
     for (size_t i = 0; i < params.pages.size(); ++i) {
@@ -156,11 +158,11 @@ bool PrintWebViewHelper::CreatePreviewDocument(
       float scale_factor = shrink;
       RenderPage(print_params, &scale_factor,
                  static_cast<int>(params.pages[i]), true, frame, &metafile);
+      page_begin_time = ReportPreviewPageRenderTime(page_begin_time);
     }
   }
 
-  UMA_HISTOGRAM_TIMES("PrintPreview.RenderToPDFTime",
-                      base::TimeTicks::Now() - begin_time);
+  base::TimeDelta render_time = base::TimeTicks::Now() - begin_time;
 
   // Ensure that printing has finished before we start cleaning up and
   // allocating buffers; this causes prep_frame_view to flush anything pending
@@ -171,11 +173,9 @@ bool PrintWebViewHelper::CreatePreviewDocument(
   if (!metafile->FinishDocument())
     NOTREACHED();
 
-  int preview_page_count = params.pages.size();
-  if (!preview_page_count)
-    preview_page_count = page_count;
-  ReportPreviewGenerationTime(base::TimeTicks::Now() - begin_time,
-                              preview_page_count);
+  ReportTotalPreviewGenerationTime(params.pages.size(), page_count,
+                                   render_time,
+                                   base::TimeTicks::Now() - begin_time);
 
   // Get the size of the compiled metafile.
   uint32 buf_size = metafile->GetDataSize();