PdfMetafileSkia gives out a SkCanvas, not a SkBaseDevice.

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..24aa12ebac8e2612bd06ab2d64e4614622bf93b9 100644
--- a/printing/pdf_metafile_skia.cc
+++ b/printing/pdf_metafile_skia.cc
@@ -10,7 +10,7 @@
 #include "base/numerics/safe_conversions.h"
 #include "base/posix/eintr_wrapper.h"
 #include "skia/ext/refptr.h"
-#include "skia/ext/vector_platform_device_skia.h"
+#include "skia/ext/vector_canvas.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/core/SkScalar.h"
@@ -33,7 +33,11 @@
 namespace printing {
 
 struct PdfMetafileSkiaData {
+  // True when finish page is outstanding for current page.
+  bool PageOutstanding() { return current_page_canvas_; }

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

Can you make it pure struct without methods?
I see no problem with checking data_->current_page_canvas_

[hal.canary, 2014/11/05 23:10:58]

Done.

+
   skia::RefPtr<SkPDFDevice> current_page_;
+  skia::RefPtr<SkCanvas> current_page_canvas_;
   SkPDFDocument pdf_doc_;
   SkDynamicMemoryWStream pdf_stream_;
 #if defined(OS_MACOSX)
@@ -51,11 +55,10 @@ bool PdfMetafileSkia::InitFromData(const void* src_buffer,
   return data_->pdf_stream_.write(src_buffer, src_buffer_size);
 }
 
-SkBaseDevice* PdfMetafileSkia::StartPageForVectorCanvas(
-    const gfx::Size& page_size, const gfx::Rect& content_area,
-    const float& scale_factor) {
-  DCHECK(!page_outstanding_);
-  page_outstanding_ = true;
+bool PdfMetafileSkia::StartPage(const gfx::Size& page_size,
+                                const gfx::Rect& content_area,
+                                const float& scale_factor) {
+  DCHECK(!data_->PageOutstanding());
 
   // Adjust for the margins and apply the scale factor.
   SkMatrix transform;
@@ -67,25 +70,29 @@ SkBaseDevice* PdfMetafileSkia::StartPageForVectorCanvas(
   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 =

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

can you just assing directly into data_->current_page_

[hal.canary, 2014/11/05 23:10:58]

Done.

-      skia::AdoptRef(new skia::VectorPlatformDeviceSkia(
+      skia::AdoptRef(new SkPDFDevice(
           pdf_page_size, pdf_content_size, transform));
   data_->current_page_ = pdf_device;
-  return pdf_device.get();
+  data_->current_page_canvas_ =
+      skia::AdoptRef(new SkCanvas(data_->current_page_.get()));
+  return true;
 }
 
-bool PdfMetafileSkia::StartPage(const gfx::Size& page_size,
-                                const gfx::Rect& content_area,
-                                const float& scale_factor) {
-  NOTREACHED();
-  return false;
+skia::VectorCanvas* PdfMetafileSkia::GetVectorCanvasForNewPage(
+    const gfx::Size& page_size, const gfx::Rect& content_area,

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

seems running "git cl format" may fix this formatting.

[hal.canary, 2014/11/05 23:10:58]

Done.

For some reason I had to `chmod +x buildtools/linux64/clang-format` before this
worked.

+    const float& scale_factor) {
+  (void)this->StartPage(page_size, content_area, scale_factor);

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

if (!StartPage(page_size, content_area, scale_factor))
  return nullptr.

[hal.canary, 2014/11/05 23:10:58]

Done.

+  return data_->current_page_canvas_.get();
 }
 
 bool PdfMetafileSkia::FinishPage() {
+  DCHECK(data_->PageOutstanding());
   DCHECK(data_->current_page_.get());
 
+  data_->current_page_canvas_.clear();  // Unref SkCanvas.
   data_->pdf_doc_.appendPage(data_->current_page_.get());
-  page_outstanding_ = false;
   return true;
 }
 
@@ -94,7 +101,7 @@ bool PdfMetafileSkia::FinishDocument() {
   if (data_->pdf_stream_.getOffset())
     return true;
 
-  if (page_outstanding_)
+  if (data_->PageOutstanding())
     FinishPage();
 
   data_->current_page_.clear();
@@ -179,6 +186,18 @@ bool PdfMetafileSkia::RenderPage(unsigned int page_number,
 }
 #endif
 
+bool PdfMetafileSkia::SaveTo(base::File* file) const {
+  DCHECK(file);

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

no need DCHECK here. It will crash on file-> anyway.

[hal.canary, 2014/11/05 23:10:58]

Done.

+  if (GetDataSize() == 0U)
+    return false;
+  SkAutoDataUnref data(data_->pdf_stream_.copyToData());
+  // TODO(halcanary): rewrite this function without extra data copy
+  // using SkStreamAsset.
+  const char* ptr = reinterpret_cast<const char*>(data->data());
+  int size = base::checked_cast<int>(data->size());
+  return file->WriteAtCurrentPos(ptr, size) == size;
+}
+
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
 bool PdfMetafileSkia::SaveToFD(const base::FileDescriptor& fd) const {
   DCHECK_GT(data_->pdf_stream_.getOffset(), 0U);
@@ -188,10 +207,7 @@ bool PdfMetafileSkia::SaveToFD(const base::FileDescriptor& fd) const {
     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);

[Vitaly Buka (NO REVIEWS), 2014/11/05 22:21:38]

no need this->

[hal.canary, 2014/11/05 23:10:58]

Done.

   DLOG_IF(ERROR, !result) << "Failed to save file with fd " << fd.fd;
 
   if (!fd.auto_close)
@@ -201,8 +217,7 @@ bool PdfMetafileSkia::SaveToFD(const base::FileDescriptor& fd) const {
 #endif
 
 PdfMetafileSkia::PdfMetafileSkia()
-    : data_(new PdfMetafileSkiaData),
-      page_outstanding_(false) {
+    : data_(new PdfMetafileSkiaData) {
 }
 
 scoped_ptr<PdfMetafileSkia> PdfMetafileSkia::GetMetafileForCurrentPage() {