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

printing/pdf_metafile_skia.cc

 // Copyright (c) 2011 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.
 
 #include "printing/pdf_metafile_skia.h"
 
 #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() {}
 
 bool PdfMetafileSkia::Init() {
   return true;
 }
 bool PdfMetafileSkia::InitFromData(const void* src_buffer,
                                    uint32 src_buffer_size) {
   return data_->pdf_stream_.write(src_buffer, src_buffer_size);
@@ skipping 27 matching lines @@
 bool PdfMetafileSkia::StartPage(const gfx::Size& page_size,
                                 const gfx::Rect& content_area,
                                 const float& scale_factor) {
   NOTREACHED();
   return NULL;
 }
 
 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;
 }
 
 bool PdfMetafileSkia::FinishDocument() {
   // Don't do anything if we've already set the data in InitFromData.
   if (data_->pdf_stream_.getOffset())
     return true;
 
   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_);
 }
 
 uint32 PdfMetafileSkia::GetDataSize() const {
   return data_->pdf_stream_.getOffset();
 }
 
 bool PdfMetafileSkia::GetData(void* dst_buffer,
                               uint32 dst_buffer_size) const {
   if (dst_buffer_size < GetDataSize())
@@ skipping 72 matching lines @@
       result = false;
     }
   }
   return result;
 }
 #endif
 
 PdfMetafileSkia::PdfMetafileSkia() : data_(new PdfMetafileSkiaData) {}
 
 }  // namespace printing