Revised PdfPsMetafile.

The interface is more reasonable, and the code is robuster.

TEST=unit test
BUG=none
Review URL: http://codereview.chromium.org/174468

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

3 files changed, 188 insertions, 130 deletions

diff --git a/printing/pdf_ps_metafile_linux.cc b/printing/pdf_ps_metafile_linux.cc
index 83c17a9..7f3027b 100644
--- a/printing/pdf_ps_metafile_linux.cc
+++ b/printing/pdf_ps_metafile_linux.cc
@@ -6,18 +6,77 @@
 
 #include <stdio.h>
 
+#include <cairo.h>
 #include <cairo-pdf.h>
 #include <cairo-ps.h>
 
 #include "base/file_util.h"
 #include "base/logging.h"
 
+namespace {
+
+// Tests if |surface| is valid.
+bool IsSurfaceValid(cairo_surface_t* surface) {
+  return cairo_surface_status(surface) == CAIRO_STATUS_SUCCESS;
+}
+
+// Tests if |context| is valid.
+bool IsContextValid(cairo_t* context) {
+  return cairo_status(context) == CAIRO_STATUS_SUCCESS;
+}
+
+// Destroys and resets |surface|.
+void CleanUpSurface(cairo_surface_t** surface) {
+  if (*surface) {
+    cairo_surface_destroy(*surface);
+    *surface = NULL;
+  }
+}
+
+// Destroys and resets |context|.
+void CleanUpContext(cairo_t** context) {
+  if (*context) {
+    cairo_destroy(*context);
+    *context = NULL;
+  }
+}
+
+// Callback function for Cairo to write PDF/PS stream.
+// |dst_buffer| is actually a pointer of type `std::string*`.
+cairo_status_t WriteCairoStream(void* dst_buffer,
+                                const unsigned char* src_data,
+                                unsigned int src_data_length) {
+  DCHECK(dst_buffer);
+  DCHECK(src_data);
+  DCHECK(src_data_length > 0);
+
+  std::string* buffer = reinterpret_cast<std::string*>(dst_buffer);
+  buffer->append(reinterpret_cast<const char*>(src_data), src_data_length);
+
+  return CAIRO_STATUS_SUCCESS;
+}
+
+}  // namespace
+
 namespace printing {
 
 PdfPsMetafile::PdfPsMetafile(const FileFormat& format)
     : format_(format),
       surface_(NULL), context_(NULL),
       page_surface_(NULL), page_context_(NULL) {
+}
+
+PdfPsMetafile::~PdfPsMetafile() {
+  // Releases resources if we forgot to do so.
+  CleanUp();
+}
+
+bool PdfPsMetafile::Init() {
+  // We need to check at least these two members to ensure Init() has not been
+  // called before. Passing these two checks also implies that surface_,
+  // page_surface_, and page_context_ are NULL, and current_page_ is empty.
+  DCHECK(!context_);
+  DCHECK(all_pages_.empty());
 
   // Create an 1 by 1 Cairo surface for entire PDF/PS file.
   // The size for each page will be overwritten later in StartPage().
@@ -36,51 +95,53 @@ PdfPsMetafile::PdfPsMetafile(const FileFormat& format)
 
     default:
       NOTREACHED();
-      return;
+      return false;
   }
 
   // Cairo always returns a valid pointer.
   // Hence, we have to check if it points to a "nil" object.
   if (!IsSurfaceValid(surface_)) {
     DLOG(ERROR) << "Cannot create Cairo surface for PdfPsMetafile!";
-    cairo_surface_destroy(surface_);
-    return;
+    CleanUpSurface(&surface_);
+    return false;
   }
 
   // Create a context.
   context_ = cairo_create(surface_);
   if (!IsContextValid(context_)) {
     DLOG(ERROR) << "Cannot create Cairo context for PdfPsMetafile!";
-    cairo_destroy(context_);
-    cairo_surface_destroy(surface_);
-    return;
+    CleanUpContext(&context_);
+    CleanUpSurface(&surface_);
+    return false;
   }
 
-  // Since |context_| will keep a reference of |surface_|, we can decreace
-  // surface's reference count by one safely.
-  cairo_surface_destroy(surface_);
+  return true;
 }
 
-PdfPsMetafile::PdfPsMetafile(const FileFormat& format,
-                             const void* src_buffer,
-                             size_t src_buffer_size)
-    : format_(format),
-      surface_(NULL), context_(NULL),
-      page_surface_(NULL), page_context_(NULL) {
+bool PdfPsMetafile::Init(const void* src_buffer, size_t src_buffer_size) {
+  // We need to check at least these two members to ensure Init() has not been
+  // called before. Passing these two checks also implies that surface_,
+  // page_surface_, and page_context_ are NULL, and current_page_ is empty.
+  DCHECK(!context_);
+  DCHECK(all_pages_.empty());
+
+  if (src_buffer == NULL || src_buffer_size == 0) {
+    return false;
+  }
 
   all_pages_ = std::string(reinterpret_cast<const char*>(src_buffer),
                            src_buffer_size);
-}
 
-PdfPsMetafile::~PdfPsMetafile() {
-  // Releases resources if we forgot to do so.
-  Close();
+  return true;
 }
 
-bool PdfPsMetafile::StartPage(double width_in_points, double height_in_points) {
+cairo_t* PdfPsMetafile::StartPage(double width_in_points,
+                                  double height_in_points) {
   DCHECK(IsSurfaceValid(surface_));
   DCHECK(IsContextValid(context_));
-  DCHECK(!page_surface_ && !page_context_);
+  // Passing this check implies page_surface_ is NULL, and current_page_ is
+  // empty.
+  DCHECK(!page_context_);
   DCHECK_GT(width_in_points, 0.);
   DCHECK_GT(height_in_points, 0.);
 
@@ -106,38 +167,35 @@ bool PdfPsMetafile::StartPage(double width_in_points, double height_in_points) {
 
     default:
       NOTREACHED();
-      return false;
+      CleanUp();
+      return NULL;
   }
 
   // Cairo always returns a valid pointer.
   // Hence, we have to check if it points to a "nil" object.
   if (!IsSurfaceValid(page_surface_)) {
     DLOG(ERROR) << "Cannot create Cairo surface for PdfPsMetafile!";
-    cairo_surface_destroy(page_surface_);
-    return false;
+    CleanUp();
+    return NULL;
   }
 
   // Create a context.
   page_context_ = cairo_create(page_surface_);
   if (!IsContextValid(page_context_)) {
     DLOG(ERROR) << "Cannot create Cairo context for PdfPsMetafile!";
-    cairo_destroy(page_context_);
-    cairo_surface_destroy(page_surface_);
-    return false;
+    CleanUp();
+    return NULL;
   }
 
-  // Since |page_context_| will keep a reference of |page_surface_|, we can
-  // decreace surface's reference count by one safely.
-  cairo_surface_destroy(page_surface_);
-
-  return true;
+  return page_context_;
 }
 
-void PdfPsMetafile::FinishPage(float shrink) {
+bool PdfPsMetafile::FinishPage(float shrink) {
   DCHECK(IsSurfaceValid(surface_));
   DCHECK(IsContextValid(context_));
   DCHECK(IsSurfaceValid(page_surface_));
   DCHECK(IsContextValid(page_context_));
+  DCHECK(shrink > 0);
 
   // Flush all rendering for current page.
   cairo_surface_flush(page_surface_);
@@ -168,7 +226,15 @@ void PdfPsMetafile::FinishPage(float shrink) {
 
     default:
       NOTREACHED();
-      return;
+      CleanUp();
+      return false;
+  }
+
+  // Check if our surface is still valid after resizing.
+  if (!IsSurfaceValid(surface_)) {
+    DLOG(ERROR) << "Cannot resize Cairo surface for PdfPsMetafile!";
+    CleanUp();
+    return false;
   }
 
   // Save context's states.
@@ -214,44 +280,68 @@ void PdfPsMetafile::FinishPage(float shrink) {
   cairo_surface_flush(surface_);
 
   // Destroy resoreces for current page.
-  cairo_destroy(page_context_);
-  page_context_ = NULL;
+  CleanUpContext(&page_context_);
   page_surface_ = NULL;
   current_page_.clear();
 
   // Restore context's states.
   cairo_restore(context_);
+
+  return true;
 }
 
 void PdfPsMetafile::Close() {
-  if (surface_ != NULL && IsSurfaceValid(surface_)) {
-    cairo_surface_finish(surface_);
-    surface_ = NULL;
-  }
-  if (context_ != NULL && IsContextValid(context_)) {
-    cairo_destroy(context_);
-    context_ = NULL;
-  }
+  DCHECK(IsSurfaceValid(surface_));
+  DCHECK(IsContextValid(context_));
+  // Passing this check implies page_surface_ is NULL, and current_page_ is
+  // empty.
+  DCHECK(!page_context_);
+
+  cairo_surface_finish(surface_);
+  DCHECK(!all_pages_.empty()); // Make sure we did get something.
+
+  CleanUpContext(&context_);
+  CleanUpSurface(&surface_);
 }
 
 unsigned int PdfPsMetafile::GetDataSize() const {
-  DCHECK(!surface_ && !context_);
+  // We need to check at least these two members to ensure that either Init()
+  // has been called to initialize |all_pages_|, or metafile has been closed.
+  // Passing these two checks also implies that surface_, page_surface_, and
+  // page_context_ are NULL, and current_page_ is empty.
+  DCHECK(!context_);
+  DCHECK(!all_pages_.empty());
 
   return all_pages_.size();
 }
 
-void PdfPsMetafile::GetData(void* dst_buffer, size_t dst_buffer_size) const {
-  DCHECK(!surface_ && !context_);
+bool PdfPsMetafile::GetData(void* dst_buffer, size_t dst_buffer_size) const {
   DCHECK(dst_buffer);
+  DCHECK(dst_buffer_size > 0);
+  // We need to check at least these two members to ensure that either Init()
+  // has been called to initialize |all_pages_|, or metafile has been closed.
+  // Passing these two checks also implies that surface_, page_surface_, and
+  // page_context_ are NULL, and current_page_ is empty.
+  DCHECK(!context_);
+  DCHECK(!all_pages_.empty());
 
   size_t data_size = GetDataSize();
-  if (data_size < dst_buffer_size)
-    dst_buffer_size = data_size;
+  if (dst_buffer_size > data_size) {
+    return false;
+  }
+
   memcpy(dst_buffer, all_pages_.data(), dst_buffer_size);
+
+  return true;
 }
 
 bool PdfPsMetafile::SaveTo(const FilePath& filename) const {
-  DCHECK(!surface_ && !context_);
+  // We need to check at least these two members to ensure that either Init()
+  // has been called to initialize |all_pages_|, or metafile has been closed.
+  // Passing these two checks also implies that surface_, page_surface_, and
+  // page_context_ are NULL, and current_page_ is empty.
+  DCHECK(!context_);
+  DCHECK(!all_pages_.empty());
 
   const unsigned int data_size = GetDataSize();
   const unsigned int bytes_written =
@@ -264,24 +354,13 @@ bool PdfPsMetafile::SaveTo(const FilePath& filename) const {
   return true;
 }
 
-cairo_status_t PdfPsMetafile::WriteCairoStream(void* dst_buffer,
-                                               const unsigned char* src_data,
-                                               unsigned int src_data_length) {
-  DCHECK(dst_buffer);
-  DCHECK(src_data);
-
-  std::string* buffer = reinterpret_cast<std::string* >(dst_buffer);
-  buffer->append(reinterpret_cast<const char*>(src_data), src_data_length);
-
-  return CAIRO_STATUS_SUCCESS;
-}
-
-bool PdfPsMetafile::IsSurfaceValid(cairo_surface_t* surface) const {
-  return cairo_surface_status(surface) == CAIRO_STATUS_SUCCESS;
-}
-
-bool PdfPsMetafile::IsContextValid(cairo_t* context) const {
-  return cairo_status(context) == CAIRO_STATUS_SUCCESS;
+void PdfPsMetafile::CleanUp() {
+  CleanUpContext(&context_);
+  CleanUpSurface(&surface_);
+  CleanUpContext(&page_context_);
+  CleanUpSurface(&page_surface_);
+  current_page_.clear();
+  all_pages_.clear();
 }
 
 }  // namespace printing
diff --git a/printing/pdf_ps_metafile_linux.h b/printing/pdf_ps_metafile_linux.h
index a49f635..bb1ac75 100644
--- a/printing/pdf_ps_metafile_linux.h
+++ b/printing/pdf_ps_metafile_linux.h
@@ -5,12 +5,13 @@
 #ifndef PRINTING_PDF_PS_METAFILE_LINUX_H_
 #define PRINTING_PDF_PS_METAFILE_LINUX_H_
 
-#include <cairo.h>
-
 #include <string>
 
 #include "base/basictypes.h"
 
+typedef struct _cairo_surface cairo_surface_t;
+typedef struct _cairo cairo_t;
+
 class FilePath;
 
 namespace printing {
@@ -24,34 +25,41 @@ class PdfPsMetafile {
     PS,
   };
 
-  // The constructor we should use in the renderer process.
+  // In the renderer process, callers should also call Init(void) to see if the
+  // metafile can obtain all necessary rendering resources.
+  // In the browser process, callers should also call Init(const void*, size_t)
+  // to initialize the buffer |all_pages_| to use SaveTo().
   explicit PdfPsMetafile(const FileFormat& format);
 
-  // The constructor we should use in the browser process.
+  ~PdfPsMetafile();
+
+  // Initializes to a fresh new metafile. Returns true on success.
+  // Note: Only call in the renderer to allocate rendering resources.
+  bool Init();
+
+  // Initializes a copy of metafile from PDF/PS stream data.
+  // Returns true on success.
   // |src_buffer| should point to the shared memory which stores PDF/PS
   // contents generated in the renderer.
-  PdfPsMetafile(const FileFormat& format,
-                const void* src_buffer,
-                size_t src_buffer_size);
+  // Note: Only call in the browser to initialize |all_pages_|.
+  bool Init(const void* src_buffer, size_t src_buffer_size);
 
-  ~PdfPsMetafile();
-
-  FileFormat GetFileFormat() { return format_; }
+  FileFormat GetFileFormat() const { return format_; }
 
   // Prepares a new cairo surface/context for rendering a new page.
-  bool StartPage(double width, double height);  // The unit is pt (=1/72 in).
-
-  // Returns the Cairo context for rendering current page.
-  cairo_t* GetPageContext() const { return page_context_; }
+  // The unit is in point (=1/72 in).
+  // Returns NULL when failed.
+  cairo_t* StartPage(double width, double height);
 
   // Destroys the surface and the context used in rendering current page.
   // The results of current page will be appended into buffer |all_pages_|.
+  // Returns true on success
   // TODO(myhuang): I plan to also do page setup here (margins, the header
   // and the footer). At this moment, only pre-defined margins for US letter
   // paper are hard-coded here.
   // |shrink| decides the scaling factor to fit raw printing results into
   // printable area.
-  void FinishPage(float shrink);
+  bool FinishPage(float shrink);
 
   // Closes resulting PDF/PS file. No further rendering is allowed.
   void Close();
@@ -62,7 +70,8 @@ class PdfPsMetafile {
 
   // Copies PDF/PS contents stored in buffer |all_pages_| into |dst_buffer|.
   // This function should ONLY be called after PDF/PS file is closed.
-  void GetData(void* dst_buffer, size_t dst_buffer_size) const;
+  // Returns true only when success.
+  bool GetData(void* dst_buffer, size_t dst_buffer_size) const;
 
   // Saves PDF/PS contents stored in buffer |all_pages_| into |filename| on
   // the disk.
@@ -70,17 +79,8 @@ class PdfPsMetafile {
   bool SaveTo(const FilePath& filename) const;
 
  private:
-  // Callback function for Cairo to write PDF/PS stream.
-  // |dst_buffer| is actually a pointer of type `std::string*`.
-  static cairo_status_t WriteCairoStream(void* dst_buffer,
-                                         const unsigned char* src_data,
-                                         unsigned int src_data_length);
-
-  // Convenient function to test if |surface| is valid.
-  bool IsSurfaceValid(cairo_surface_t* surface) const;
-
-  // Convenient function to test if |context| is valid.
-  bool IsContextValid(cairo_t* context) const;
+  // Cleans up all resources.
+  void CleanUp();
 
   FileFormat format_;
 
@@ -104,4 +104,3 @@ class PdfPsMetafile {
 }  // namespace printing
 
 #endif  // PRINTING_PDF_PS_METAFILE_LINUX_H_
-
diff --git a/printing/pdf_ps_metafile_linux_unittest.cc b/printing/pdf_ps_metafile_linux_unittest.cc
index 8728ba3..3ef0822 100644
--- a/printing/pdf_ps_metafile_linux_unittest.cc
+++ b/printing/pdf_ps_metafile_linux_unittest.cc
@@ -15,31 +15,22 @@ typedef struct _cairo cairo_t;
 TEST(PdfTest, Basic) {
   // Tests in-renderer constructor.
   printing::PdfPsMetafile pdf(printing::PdfPsMetafile::PDF);
-  cairo_t* context = pdf.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(pdf.Init());
 
   // Renders page 1.
-  EXPECT_TRUE(pdf.StartPage(72, 72));
-  context = pdf.GetPageContext();
+  cairo_t* context = pdf.StartPage(72, 72);
   EXPECT_TRUE(context != NULL);
   // In theory, we should use Cairo to draw something on |context|.
-  pdf.FinishPage(1.5);
-  context = pdf.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(pdf.FinishPage(1.5));
 
   // Renders page 2.
-  EXPECT_TRUE(pdf.StartPage(64, 64));
-  context = pdf.GetPageContext();
+  context = pdf.StartPage(64, 64);
   EXPECT_TRUE(context != NULL);
   // In theory, we should use Cairo to draw something on |context|.
-  pdf.FinishPage(0.5);
-  context = pdf.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(pdf.FinishPage(0.5));
 
   // Closes the file.
   pdf.Close();
-  context = pdf.GetPageContext();
-  EXPECT_TRUE(context == NULL);
 
   // Checks data size.
   unsigned int size = pdf.GetDataSize();
@@ -50,9 +41,8 @@ TEST(PdfTest, Basic) {
   pdf.GetData(&buffer.front(), size);
 
   // Tests another constructor.
-  printing::PdfPsMetafile pdf2(printing::PdfPsMetafile::PDF,
-                               &buffer.front(),
-                               size);
+  printing::PdfPsMetafile pdf2(printing::PdfPsMetafile::PDF);
+  EXPECT_TRUE(pdf2.Init(&buffer.front(), size));
 
   // Tries to get the first 4 characters from pdf2.
   std::vector<char> buffer2(4, 0x00);
@@ -69,31 +59,22 @@ TEST(PdfTest, Basic) {
 TEST(PsTest, Basic) {
   // Tests in-renderer constructor.
   printing::PdfPsMetafile ps(printing::PdfPsMetafile::PS);
-  cairo_t* context = ps.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(ps.Init());
 
   // Renders page 1.
-  EXPECT_TRUE(ps.StartPage(72, 72));
-  context = ps.GetPageContext();
+  cairo_t* context = ps.StartPage(72, 72);
   EXPECT_TRUE(context != NULL);
   // In theory, we should use Cairo to draw something on |context|.
-  ps.FinishPage(1.5);
-  context = ps.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(ps.FinishPage(1.5));
 
   // Renders page 2.
-  EXPECT_TRUE(ps.StartPage(64, 64));
-  context = ps.GetPageContext();
+  context = ps.StartPage(64, 64);
   EXPECT_TRUE(context != NULL);
   // In theory, we should use Cairo to draw something on |context|.
-  ps.FinishPage(0.5);
-  context = ps.GetPageContext();
-  EXPECT_TRUE(context == NULL);
+  EXPECT_TRUE(ps.FinishPage(0.5));
 
   // Closes the file.
   ps.Close();
-  context = ps.GetPageContext();
-  EXPECT_TRUE(context == NULL);
 
   // Checks data size.
   unsigned int size = ps.GetDataSize();
@@ -104,9 +85,8 @@ TEST(PsTest, Basic) {
   ps.GetData(&buffer.front(), size);
 
   // Tests another constructor.
-  printing::PdfPsMetafile ps2(printing::PdfPsMetafile::PS,
-                              &buffer.front(),
-                              size);
+  printing::PdfPsMetafile ps2(printing::PdfPsMetafile::PS);
+  EXPECT_TRUE(ps2.Init(&buffer.front(), size));
 
   // Tries to get the first 4 characters from ps2.
   std::vector<char> buffer2(4, 0x00);