Add metrics to determine what font types are being used with print preview.

printing/pdf_metafile_skia.cc

Index: printing/pdf_metafile_skia.cc
diff --git a/printing/pdf_metafile_skia.cc b/printing/pdf_metafile_skia.cc
index f76eb704f0d712ec590babecb6be2db7437032bd..2558a8453a95f34229ee906e3bb17470a90b130e 100644
--- a/printing/pdf_metafile_skia.cc
+++ b/printing/pdf_metafile_skia.cc
@@ -7,23 +7,33 @@
 #include "base/eintr_wrapper.h"
 #include "base/file_descriptor_posix.h"
 #include "base/file_util.h"
+#include "base/hash_tables.h"
+#include "base/metrics/histogram.h"
 #include "skia/ext/vector_platform_device_skia.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/core/SkScalar.h"
 #include "third_party/skia/include/core/SkStream.h"
+#include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/pdf/SkPDFDevice.h"
 #include "third_party/skia/include/pdf/SkPDFDocument.h"
+#include "third_party/skia/include/pdf/SkPDFFont.h"
 #include "third_party/skia/include/pdf/SkPDFPage.h"
 #include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
 
+namespace {
+  typedef base::hash_map<SkFontID, SkAdvancedTypefaceMetrics::FontType>
+      FontTypeMap;
+};
+
 namespace printing {
 
 struct PdfMetafileSkiaData {
   SkRefPtr<SkPDFDevice> current_page_;
   SkPDFDocument pdf_doc_;
   SkDynamicMemoryWStream pdf_stream_;
+  FontTypeMap font_type_stats_;
 };
 
 PdfMetafileSkia::~PdfMetafileSkia() {}
@@ -71,6 +81,13 @@ bool PdfMetafileSkia::StartPage(const gfx::Size& page_size,
 bool PdfMetafileSkia::FinishPage() {
   DCHECK(data_->current_page_.get());
 
+  const SkTDArray<SkPDFFont*>& font_resources =

[Chris Guillory, 2011/05/29 01:50:48]

Can we not put this logic into FinishDocument() somehow?

[vandebo (ex-Chrome), 2011/05/29 06:14:54]

We could keep a list of pages instead of just the current page... or we could
add an accessor to get the list of pages out of the Document, but I'm not sure
what that will gain us.  Can you say more?

[Chris Guillory, 2011/05/30 16:46:31]

For a second I was still thinking that the document had a way to access the
fonts but it looks it doesn't.
http://codereview.appspot.com/4547069/patch/1/7

Optional: I think an accessor for the pages makes sense.
* All this logic could be together inside of FinishDocuemnt.
* We wouldn't need to add the font_type_stats_ member.

+      data_->current_page_->getFontResources();
+  for (int i = 0; i < font_resources.count(); i++) {
+    SkFontID key = font_resources[i]->typeface()->uniqueID();
+    data_->font_type_stats_[key] = font_resources[i]->getType();
+  }
+
   data_->pdf_doc_.appendPage(data_->current_page_);
   data_->current_page_ = NULL;
   return true;
@@ -83,6 +100,16 @@ bool PdfMetafileSkia::FinishDocument() {
 
   if (data_->current_page_.get())
     FinishPage();
+
+  for (FontTypeMap::const_iterator it = data_->font_type_stats_.begin();
+       it != data_->font_type_stats_.end();
+       it++) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "PrintPreview.FontType",
+        it->second,
+        SkAdvancedTypefaceMetrics::kNotEmbeddable_Font + 1);

[jar (doing other things), 2011/05/29 01:49:20]

There are some interesting biases you have in this, and you should be sure
you're doing this intentionally.  IMO, you may be adding complexity to the code
and size to data (using a hashmap), to further enhance the bias.

There are only about 7 distinct font types.  If a page has a dozen distinct
fonts, and they are all the same "FontType," do you really want to count that 12
times?  I'm guessing this code already counts pages that reload themselves more
than ones that don't (a harder bias to avoid).  IMO, it might be interesting to
know if a page has *any* fonts of a given type, but it is less significant that
it had dozens of a given FontType.... but then again, I'm not sure what you're
intending to conclude from the gathered stats.

Perhaps the nicest thing would be (some day) to have (for this histogram) a list
of how many users participated in use of each FontType.  Perhaps that will be
the most interesting metric, and that will de-dup the counting completely.

For now, I *suspect* you really only want to answer as to whether a given page
has a FontType.  Hence I'd *guess* you'd be better off maintaining a small
vector of bools, which you aggregate into the histogram here, than maintaining a
hash_map.

+  }
+
   return data_->pdf_doc_.emitPDF(&data_->pdf_stream_);
 }