Remove calls to deprecated SkPDFDevice and SkPDFDocuemnt.

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 36a42d9d135cc875e803c8f01a3c2b5b048d5e8c..35df8258eeefcc07aa2861fbf736c85107a7568a 100644
--- a/printing/pdf_metafile_skia.cc
+++ b/printing/pdf_metafile_skia.cc
@@ -9,15 +9,17 @@
 #include "base/metrics/histogram.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/posix/eintr_wrapper.h"
+#include "skia/ext/platform_canvas.h"
 #include "skia/ext/refptr.h"
-#include "skia/ext/vector_platform_device_skia.h"
-#include "third_party/skia/include/core/SkData.h"
+#include "skia/ext/vector_canvas.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkDocument.h"
+#include "third_party/skia/include/core/SkPictureRecorder.h"
+#include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/core/SkScalar.h"
+#include "third_party/skia/include/core/SkSize.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 "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
@@ -30,115 +32,175 @@
 #include "base/file_descriptor_posix.h"
 #endif
 
+class SkBaseDevice;
+
+namespace {
+// This struct represents all the data we need to draw and redraw this
+// page into a SkDocument.
+struct Page {
+  Page(const SkSize& page_size, const SkRect& content_area);
+  SkSize page_size_;
+  SkRect content_area_;
+  skia::RefPtr<SkPicture> content_;
+};
+Page::Page(const SkSize& page_size, const SkRect& content_area)
+    : page_size_(page_size), content_area_(content_area), content_(/*NULL*/) {}
+}  // namespace
+
 namespace printing {
 
 struct PdfMetafileSkiaData {
-  skia::RefPtr<SkPDFDevice> current_page_;
-  SkPDFDocument pdf_doc_;
-  SkDynamicMemoryWStream pdf_stream_;
+  scoped_ptr<SkPictureRecorder> recorder_;  // Current recording
+
+  std::vector<Page> pages_;
+  skia::RefPtr<SkStreamAsset> pdf_data_;
+
+  // True when the current page is outstanding (and still writable).
+  bool PageOutstanding();
+
+  // True if InitFromData() or FinishDocument has been called.
+  bool HasData();
+
 #if defined(OS_MACOSX)
   PdfMetafileCg pdf_cg_;
 #endif
 };
 
+bool PdfMetafileSkiaData::HasData() { return (pdf_data_.get() != NULL); }
+bool PdfMetafileSkiaData::PageOutstanding() { return recorder_.get() != NULL; }
+
 PdfMetafileSkia::~PdfMetafileSkia() {}
 
 bool PdfMetafileSkia::Init() {
   return true;
 }
+
+// TODO(halcanary): Create a Metafile class that only stores data.
+// Metafile::InitFromData is orthogonal to what the rest of
+// PdfMetafileSkia does.
 bool PdfMetafileSkia::InitFromData(const void* src_buffer,
                                    uint32 src_buffer_size) {
-  return data_->pdf_stream_.write(src_buffer, src_buffer_size);
+  if (data_->HasData()) data_->pdf_data_.clear();  // free up RAM first.
+  SkDynamicMemoryWStream dynamic_memory;
+  if (!dynamic_memory.write(src_buffer, src_buffer_size)) return false;
+  data_->pdf_data_ = skia::AdoptRef(dynamic_memory.detachAsStream());
+  return true;
+}
+
+skia::RefPtr<skia::VectorCanvas> PdfMetafileSkia::GetVectorCanvasForNewPage(
+    const gfx::Size& page_size, const gfx::Rect& content_area,
+    const float& scale_factor) {
+  return this->StartPage(page_size, content_area, scale_factor)
+             ? skia::SharePtr(data_->recorder_->getRecordingCanvas())
+             : skia::RefPtr<skia::VectorCanvas>(/*NULL*/);
 }
 
 SkBaseDevice* PdfMetafileSkia::StartPageForVectorCanvas(
     const gfx::Size& page_size, const gfx::Rect& content_area,
     const float& scale_factor) {

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

Why not just kill this code.

[hal.canary, 2014/11/04 15:30:35]

 The Metafile interface requires it.

[Vitaly Buka (NO REVIEWS), 2014/11/04 19:23:55]

https://codereview.chromium.org/697373004/

-  DCHECK(!page_outstanding_);
-  page_outstanding_ = true;
-
-  // Adjust for the margins and apply the scale factor.
-  SkMatrix transform;
-  transform.setTranslate(SkIntToScalar(content_area.x()),
-                         SkIntToScalar(content_area.y()));
-  transform.preScale(SkFloatToScalar(scale_factor),
-                     SkFloatToScalar(scale_factor));
-
-  SkISize pdf_page_size = SkISize::Make(page_size.width(), page_size.height());
-  SkISize pdf_content_size =
-      SkISize::Make(content_area.width(), content_area.height());
-  skia::RefPtr<SkPDFDevice> pdf_device =
-      skia::AdoptRef(new skia::VectorPlatformDeviceSkia(
-          pdf_page_size, pdf_content_size, transform));
-  data_->current_page_ = pdf_device;
-  return pdf_device.get();
+  NOTREACHED();  // Skia has deprecated the use of SkPDFDevice.
+  return NULL;
+}
+
+inline SkSize to_sk_size(const gfx::Size& size) {
+  return SkSize::Make(SkIntToScalar(size.width()),

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

to_sk_size -> ToSkSize

[hal.canary, 2014/11/04 15:30:34]

Done.

+                      SkIntToScalar(size.height()));

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

please put in namespace {} in front pf namespace printing {
and remove inline, usually we don't use them

[hal.canary, 2014/11/04 15:30:34]

Done.

+}
+
+inline SkRect to_sk_rect(const gfx::Rect& rect) {

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

fix name

[hal.canary, 2014/11/04 15:30:34]

Done.

+  return SkRect::MakeLTRB(SkIntToScalar(rect.x()), SkIntToScalar(rect.y()),
+                          SkIntToScalar(rect.right()),
+                          SkIntToScalar(rect.bottom()));

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

same

[hal.canary, 2014/11/04 15:30:35]

Done.

 }
 
 bool PdfMetafileSkia::StartPage(const gfx::Size& page_size,
                                 const gfx::Rect& content_area,
                                 const float& scale_factor) {
-  NOTREACHED();
-  return false;
+  if (data_->PageOutstanding()) {

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

no need {}

[hal.canary, 2014/11/04 15:30:34]

Done.

+    this->FinishPage();
+  }
+
+  data_->recorder_.reset(SkNEW(SkPictureRecorder));
+  SkSize sk_page_size = to_sk_size(page_size);
+  data_->pages_.push_back(Page(sk_page_size, to_sk_rect(content_area)));
+
+  SkCanvas* recordingCanvas = data_->recorder_->beginRecording(
+      sk_page_size.width(), sk_page_size.height(), NULL, 0);
+  // recordingCanvas is owned by the data_->recorder_.  No ref() necessary.
+  if (NULL == recordingCanvas) {

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

if (!recordingCanvas) {

[hal.canary, 2014/11/04 15:30:34]

Done.

+    return false;
+  }
+  recordingCanvas->scale(scale_factor, scale_factor);
+  return true;
 }
 
 bool PdfMetafileSkia::FinishPage() {
-  DCHECK(data_->current_page_.get());
-
-  data_->pdf_doc_.appendPage(data_->current_page_.get());
-  page_outstanding_ = false;
+  if (!data_->PageOutstanding()) return false;

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

please move returns on separate line

also please run "git cl format"

[hal.canary, 2014/11/04 15:30:34]

Done.

+  DCHECK(data_->pages_.back().content_.get() == NULL);

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

DCHECK_EQ

[hal.canary, 2014/11/04 15:30:34]

Done.

+  data_->pages_.back().content_ =
+      skia::AdoptRef(data_->recorder_->endRecording());
+  data_->recorder_.reset();
   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 (page_outstanding_)
-    FinishPage();
-
-  data_->current_page_.clear();
-
-  int font_counts[SkAdvancedTypefaceMetrics::kOther_Font + 2];
-  data_->pdf_doc_.getCountOfFontTypes(font_counts);
-  for (int type = 0;
-       type <= SkAdvancedTypefaceMetrics::kOther_Font + 1;
-       type++) {
-    for (int count = 0; count < font_counts[type]; count++) {
-      UMA_HISTOGRAM_ENUMERATION(
-          "PrintPreview.FontType", type,
-          SkAdvancedTypefaceMetrics::kOther_Font + 2);
-    }
+  // If we've already set the data in InitFromData, overwrite it.
+  if (data_->HasData()) data_->pdf_data_.clear();  // free us RAM first/

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

put if body on the separate line

[hal.canary, 2014/11/04 15:30:34]

Done.

+
+  if (data_->PageOutstanding()) FinishPage();
+
+  // std::vector<skia::RefPtr<SkPicture>> pages_;

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

remove comment

[hal.canary, 2014/11/04 15:30:34]

Done.

+  SkDynamicMemoryWStream pdf_stream;
+  skia::RefPtr<SkDocument> pdf_doc =
+      skia::AdoptRef(SkDocument::CreatePDF(&pdf_stream));
+  for (const auto& page : data_->pages_) {
+    SkCanvas* canvas =
+        pdf_doc->beginPage(page.page_size_.width(), page.page_size_.height(),
+                           &(page.content_area_));
+    canvas->drawPicture(page.content_.get());
+    pdf_doc->endPage();
   }
+  if (!pdf_doc->close()) return false;
+  pdf_doc.clear();

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

why pdf_doc.clear(); is needed?

[hal.canary, 2014/11/04 15:30:34]

removed

 
-  return data_->pdf_doc_.emitPDF(&data_->pdf_stream_);
+  data_->pdf_data_ = skia::AdoptRef(pdf_stream.detachAsStream());
+  return true;
 }
 
 uint32 PdfMetafileSkia::GetDataSize() const {
-  return base::checked_cast<uint32>(data_->pdf_stream_.getOffset());
+  if (data_->pdf_data_.get() == NULL) return 0;

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

RefPtr has bool operator, so please just put if (!data_->pdf_data_)

[hal.canary, 2014/11/04 15:30:34]

Done.

+  return base::checked_cast<uint32>(data_->pdf_data_->getLength());
+}
+
+inline bool copy_asset(SkStreamAsset* asset, void* dst, size_t size) {

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

copy_asset -> CopyAsset

[hal.canary, 2014/11/04 15:30:34]

Done.

+  DCHECK(asset);

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

move all utilities into namespace {} and remove inlines

[hal.canary, 2014/11/04 15:30:34]

Done.

+  DCHECK(asset->getPosition() == 0);
+  size_t length = asset->getLength();
+  if (length > size) return false;
+  bool success = (length == asset->read(dst, length));
+  (void)asset->rewind();
+  return success;
 }
 
 bool PdfMetafileSkia::GetData(void* dst_buffer,
                               uint32 dst_buffer_size) const {
-  if (dst_buffer_size < GetDataSize())
-    return false;
+  if (data_->pdf_data_.get() == NULL) return false;
+  return copy_asset(data_->pdf_data_.get(), dst_buffer,
+                    base::checked_cast<size_t>(dst_buffer_size));
+}
 
-  SkAutoDataUnref data(data_->pdf_stream_.copyToData());
-  memcpy(dst_buffer, data->bytes(), dst_buffer_size);
-  return true;
+static gfx::Rect to_gfx_rect(const SkRect& rect) {
+  return gfx::Rect(SkScalarTruncToInt(rect.x()), SkScalarTruncToInt(rect.y()),
+                   SkScalarTruncToInt(rect.width()),
+                   SkScalarTruncToInt(rect.height()));
 }
 
 gfx::Rect PdfMetafileSkia::GetPageBounds(unsigned int page_number) const {
-  // TODO(vandebo) add a method to get the page size for a given page to
-  // SkPDFDocument.
-  NOTIMPLEMENTED();
-  return gfx::Rect();
+  return to_gfx_rect(data_->pages_[page_number].content_area_);
 }
 
 unsigned int PdfMetafileSkia::GetPageCount() const {
-  // TODO(vandebo) add a method to get the number of pages to SkPDFDocument.
-  NOTIMPLEMENTED();
-  return 0;
+  return base::checked_cast<int>(data_->pages_.size());
 }
 
 gfx::NativeDrawingContext PdfMetafileSkia::context() const {
@@ -170,28 +232,53 @@ bool PdfMetafileSkia::RenderPage(unsigned int page_number,
                                  CGContextRef context,
                                  const CGRect rect,
                                  const MacRenderPageParams& params) const {
-  DCHECK_GT(data_->pdf_stream_.getOffset(), 0U);
+  DCHECK_GT(GetDataSize(), 0U);
   if (data_->pdf_cg_.GetDataSize() == 0) {
-    SkAutoDataUnref data(data_->pdf_stream_.copyToData());
-    data_->pdf_cg_.InitFromData(data->bytes(), data->size());
+    if (GetDataSize() == 0) return false;
+    size_t length = data_->pdf_data_->getLength();
+    scoped_ptr<uint8_t[]> buffer(new uint8_t[length]);
+    (void)copy_asset(data_->pdf_data_.get(), buffer.get(), length);
+    data_->pdf_cg_.InitFromData(buffer.get(),
+                                base::checked_cast<uint32>(length));
   }
   return data_->pdf_cg_.RenderPage(page_number, context, rect, params);
 }
 #endif
 
+inline bool copy_to_file(SkStreamAsset* asset, base::File* file) {
+  DCHECK(asset);
+  DCHECK(file);
+  DCHECK_EQ(asset->getPosition(), 0U);
+  char buffer[4096];
+  do {
+    size_t read_size = asset->read(buffer, sizeof(buffer));
+    DCHECK_GE(sizeof(buffer), read_size);
+    if (!file->WriteAtCurrentPos(buffer, base::checked_cast<int>(read_size))) {
+      (void)asset->rewind();
+      return false;
+    }
+  } while (!asset->isAtEnd());
+  (void)asset->rewind();
+  return true;
+}
+
+bool PdfMetafileSkia::SaveTo(base::File* file) const {
+  if (GetDataSize() == 0U) return false;
+  DCHECK(data_->pdf_data_.get());
+  return copy_to_file(data_->pdf_data_.get(), file);
+}
+
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
+
 bool PdfMetafileSkia::SaveToFD(const base::FileDescriptor& fd) const {
-  DCHECK_GT(data_->pdf_stream_.getOffset(), 0U);
+  DCHECK_GT(GetDataSize(), 0U);
 
   if (fd.fd < 0) {
     DLOG(ERROR) << "Invalid file descriptor!";
     return false;
   }
   base::File file(fd.fd);
-  SkAutoDataUnref data(data_->pdf_stream_.copyToData());
-  bool result =
-      file.WriteAtCurrentPos(reinterpret_cast<const char*>(data->data()),
-                             GetDataSize()) == static_cast<int>(GetDataSize());
+  bool result = this->SaveTo(&file);
   DLOG_IF(ERROR, !result) << "Failed to save file with fd " << fd.fd;
 
   if (!fd.auto_close)
@@ -200,28 +287,47 @@ bool PdfMetafileSkia::SaveToFD(const base::FileDescriptor& fd) const {
 }
 #endif
 
-PdfMetafileSkia::PdfMetafileSkia()
-    : data_(new PdfMetafileSkiaData),
-      page_outstanding_(false) {
+PdfMetafileSkia::PdfMetafileSkia() : data_(new PdfMetafileSkiaData) {
+  DCHECK(!data_->PageOutstanding());
+}
+
+// Return a const pointer to the page.
+// The page must be finished (via FinishPage()).
+// Return NULL on error.
+static const Page* get_page(PdfMetafileSkiaData* input_data,

[Vitaly Buka (NO REVIEWS), 2014/11/03 19:57:02]

remove static and move into namespace {}

[hal.canary, 2014/11/04 15:30:34]

I made it a method of PdfMetafileSkiaData.

+                            size_t page_number) {
+  DCHECK(input_data);
+  if (page_number >= input_data->pages_.size()) {
+    return NULL;
+  }
+  if (page_number == input_data->pages_.size() - 1) {
+    // Last page.
+    if (input_data->PageOutstanding()) {
+      // page not finished recording
+      return NULL;
+    }
+  }
+  DCHECK(input_data->pages_[page_number].content_.get() != NULL);
+  return &(input_data->pages_[page_number]);
 }
 
 scoped_ptr<PdfMetafileSkia> PdfMetafileSkia::GetMetafileForCurrentPage() {
-  scoped_ptr<PdfMetafileSkia> metafile;
-  SkPDFDocument pdf_doc(SkPDFDocument::kDraftMode_Flags);
-  if (!pdf_doc.appendPage(data_->current_page_.get()))
+  // If we only ever need the metafile for the last page, should we
+  // only keep a handle one one SkPicture?
+  DCHECK_NE(data_->pages_.size(), 0U);
+  DCHECK(!data_->PageOutstanding());
+  scoped_ptr<PdfMetafileSkia> metafile(new PdfMetafileSkia);
+  if (data_->pages_.size() == 0) {
     return metafile.Pass();
-
-  SkDynamicMemoryWStream pdf_stream;
-  if (!pdf_doc.emitPDF(&pdf_stream))
+  }
+  size_t page_number = data_->pages_.size() - 1;
+  const Page* page = get_page(data_.get(), page_number);
+  if (!page) {
     return metafile.Pass();
-
-  SkAutoDataUnref data_copy(pdf_stream.copyToData());
-  if (data_copy->size() == 0)
-    return scoped_ptr<PdfMetafileSkia>();
-
-  metafile.reset(new PdfMetafileSkia);
-  if (!metafile->InitFromData(data_copy->bytes(),
-                              base::checked_cast<uint32>(data_copy->size()))) {
+  }
+  metafile->data_->pages_.push_back(*page);  // Copy page data;
+  // Should increment refcnt on page->content_.
+  if (!metafile->FinishDocument()) {  // Generate PDF.
     metafile.reset();
   }
   return metafile.Pass();