Unifying StartPage across all platforms

Since there is already a getter method for retrieving the context, there is no reason for StartPage to return it. Also the parameter list can be uniform across all platforms. Some arguments are unused in order to achieve this. BUG=NONE TEST=NONE Review URL: http://codereview.chromium.org/6667069 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78717 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Mar 18, 2011 · edc531f · edc531f
1 parent 3e7b5d6
commit edc531f
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 60 deletions.
diff --git a/chrome/renderer/print_web_view_helper_linux.cc b/chrome/renderer/print_web_view_helper_linux.cc
@@ -13,6 +13,7 @@
 #include "printing/native_metafile.h"
 #include "skia/ext/vector_canvas.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
+#include "ui/gfx/point.h"
 
 #if !defined(OS_CHROMEOS)
 #include "base/process_util.h"
@@ -220,14 +221,12 @@ void PrintWebViewHelper::PrintPage(const ViewMsg_PrintPage_Params& params,
       content_height_in_points + margin_top_in_points +
           margin_bottom_in_points);
 
-  cairo_t* cairo_context =
-      metafile->StartPage(page_size,
-                          margin_top_in_points,
-                          margin_left_in_points);
-  if (!cairo_context)
+  gfx::Point content_origin(margin_top_in_points, margin_left_in_points);
+
+  if (!metafile->StartPage(page_size, content_origin, 1))
     return;
 
-  canvas->reset(new skia::VectorCanvas(cairo_context,
+  canvas->reset(new skia::VectorCanvas(metafile->context(),
                                        canvas_size.width(),
                                        canvas_size.height()));
   frame->printPage(params.page_number, canvas->get());

diff --git a/chrome/renderer/print_web_view_helper_mac.mm b/chrome/renderer/print_web_view_helper_mac.mm
@@ -106,15 +106,14 @@ PrepareFrameAndViewForPrint prep_frame_view(printParams,
     const gfx::Size& page_size, const gfx::Point& content_origin,
     const float& scale_factor, int page_number, WebFrame* frame,
     printing::NativeMetafile* metafile) {
-  CGContextRef context = metafile->StartPage(page_size, content_origin,
-                                             scale_factor);
-  DCHECK(context);
+  bool success = metafile->StartPage(page_size, content_origin, scale_factor);
+  DCHECK(success);
 
   // printPage can create autoreleased references to |context|. PDF contexts
   // don't write all their data until they are destroyed, so we need to make
   // certain that there are no lingering references.
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
-  frame->printPage(page_number, context);
+  frame->printPage(page_number, metafile->context());
   [pool release];
 
   // Done printing. Close the device context to retrieve the compiled metafile.

diff --git a/chrome/renderer/print_web_view_helper_win.cc b/chrome/renderer/print_web_view_helper_win.cc
@@ -15,6 +15,9 @@
 #include "skia/ext/vector_platform_device.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "ui/gfx/gdi_util.h"
+#include "ui/gfx/point.h"
+#include "ui/gfx/rect.h"
+#include "ui/gfx/size.h"
 
 using printing::ConvertUnitDouble;
 using printing::kPointsPerInch;
@@ -199,17 +202,25 @@ void PrintWebViewHelper::RenderPage(
 
   double content_width_in_points;
   double content_height_in_points;
+  double margin_top_in_points;
+  double margin_left_in_points;
   GetPageSizeAndMarginsInPoints(frame, page_number, params,
-      &content_width_in_points, &content_height_in_points, NULL, NULL, NULL,
-      NULL);
+                                &content_width_in_points,
+                                &content_height_in_points,
+                                &margin_top_in_points, NULL, NULL,
+                                &margin_left_in_points);
 
   // Since WebKit extends the page width depending on the magical scale factor
   // we make sure the canvas covers the worst case scenario (x2.0 currently).
   // PrintContext will then set the correct clipping region.
   int width = static_cast<int>(content_width_in_points * params.max_shrink);
   int height = static_cast<int>(content_height_in_points * params.max_shrink);
 
-  bool result = (*metafile)->StartPage();
+  bool result = (*metafile)->StartPage(
+      gfx::Size(width, height),
+      gfx::Point(static_cast<int>(margin_top_in_points),
+                 static_cast<int>(margin_left_in_points)),
+      *scale_factor);
   DCHECK(result);
 
 #if 0

diff --git a/chrome/utility/utility_thread.cc b/chrome/utility/utility_thread.cc
@@ -22,7 +22,9 @@
 #include "printing/units.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSerializedScriptValue.h"
+#include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
+#include "ui/gfx/size.h"
 #include "webkit/glue/idb_bindings.h"
 #include "webkit/glue/image_decoder.h"
 
@@ -276,7 +278,9 @@ bool UtilityThread::RenderPDFToWinMetafile(
     for (int page_number = iter->from; page_number <= iter->to; ++page_number) {
       if (page_number >= total_page_count)
         break;
-      metafile->StartPage();
+      // The underlying metafile is of type Emf and ignores the arguments passed
+      // to StartPage.
+      metafile->StartPage(gfx::Size(), gfx::Point(), 1);
       if (render_proc(&buffer.front(), buffer.size(), page_number,
                       metafile->context(), render_dpi, render_dpi,
                       render_area.x(), render_area.y(), render_area.width(),

diff --git a/printing/emf_win.cc b/printing/emf_win.cc
@@ -13,7 +13,9 @@
 #include "ui/gfx/codec/jpeg_codec.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/gdi_util.h"
+#include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
+#include "ui/gfx/size.h"
 
 namespace {
 const int kCustomGdiCommentSignature = 0xdeadbabe;
@@ -410,7 +412,10 @@ bool Emf::Record::SafePlayback(const XFORM* base_matrix) const {
   return res;
 }
 
-bool Emf::StartPage() {
+bool Emf::StartPage(const gfx::Size& /*page_size*/,
+                    const gfx::Point& /*content_origin*/,
+                    const float& scale_factor) {
+  DCHECK_EQ(scale_factor, 1);
   DCHECK(hdc_);
   if (!hdc_)
     return false;

diff --git a/printing/emf_win.h b/printing/emf_win.h
@@ -15,7 +15,9 @@
 class FilePath;
 
 namespace gfx {
+class Point;
 class Rect;
+class Size;
 }
 
 namespace printing {
@@ -33,7 +35,13 @@ class Emf : public NativeMetafile {
   virtual bool Init() { return true; }
   virtual bool InitFromData(const void* src_buffer, uint32 src_buffer_size);
 
-  virtual bool StartPage();
+  // Inserts a custom GDICOMMENT records indicating StartPage/EndPage calls
+  // (since StartPage and EndPage do not work in a metafile DC). Only valid
+  // when hdc_ is non-NULL. |page_size| and |content_origin| are ignored.
+  // |scale_factor|  must be 1.0.
+  virtual bool StartPage(const gfx::Size& page_size,
+                         const gfx::Point& content_origin,
+                         const float& scale_factor);
   virtual bool FinishPage();
   virtual bool Close();
 

diff --git a/printing/emf_win_unittest.cc b/printing/emf_win_unittest.cc
@@ -17,6 +17,8 @@
 #include "base/win/scoped_hdc.h"
 #include "printing/printing_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/point.h"
+#include "ui/gfx/size.h"
 
 namespace {
 
@@ -131,7 +133,7 @@ TEST_F(EmfPrintingTest, PageBreak) {
   EXPECT_TRUE(emf.context() != NULL);
   int pages = 3;
   while (pages) {
-    EXPECT_TRUE(emf.StartPage());
+    EXPECT_TRUE(emf.StartPage(gfx::Size(), gfx::Point(), 1));
     ::Rectangle(emf.context(), 10, 10, 190, 190);
     EXPECT_TRUE(emf.FinishPage());
     --pages;

diff --git a/printing/native_metafile.h b/printing/native_metafile.h
@@ -21,9 +21,9 @@
 class FilePath;
 
 namespace gfx {
+class Point;
 class Rect;
 class Size;
-class Point;
 }
 
 #if defined(OS_CHROMEOS)
@@ -50,26 +50,12 @@ class NativeMetafile {
   // Note: It should only be called from within the browser process.
   virtual bool InitFromData(const void* src_buffer, uint32 src_buffer_size) = 0;
 
-#if defined(OS_WIN)
-  // Inserts a custom GDICOMMENT records indicating StartPage/EndPage calls
-  // (since StartPage and EndPage do not work in a metafile DC). Only valid
-  // when hdc_ is non-NULL.
-  virtual bool StartPage() = 0;
-#elif defined(OS_MACOSX)
-  // Prepares a new pdf page at specified |content_origin| with the given
-  // |page_size| and a |scale_factor| to use for the drawing.
-  virtual gfx::NativeDrawingContext StartPage(const gfx::Size& page_size,
-                                              const gfx::Point& content_origin,
-                                              const float& scale_factor) = 0;
-#elif defined(OS_POSIX)
-  // Prepares a new cairo surface/context for rendering a new page.
-  // The unit is in point (=1/72 in).
-  // Returns NULL when failed.
-  virtual gfx::NativeDrawingContext StartPage(const gfx::Size& page_size,
-                                              double margin_top_in_points,
-                                              double margin_left_in_points) = 0;
-#endif
-
+  // Prepares a context for rendering a new page at the specified
+  // |content_origin| with the given |page_size| and a |scale_factor| to use for
+  // the drawing. The units are in points (=1/72 in). Returns true on success.
+  virtual bool StartPage(const gfx::Size& page_size,
+                         const gfx::Point& content_origin,
+                         const float& scale_factor) = 0;
 
   // Closes the current page and destroys the context used in rendering that
   // page. The results of current page will be appended into the underlying

diff --git a/printing/pdf_metafile_mac.cc b/printing/pdf_metafile_mac.cc
@@ -62,8 +62,9 @@ bool PdfMetafile::InitFromData(const void* src_buffer, uint32 src_buffer_size) {
   return true;
 }
 
-CGContextRef PdfMetafile::StartPage(const gfx::Size& page_size,
-    const gfx::Point& content_origin, const float& scale_factor) {
+bool PdfMetafile::StartPage(const gfx::Size& page_size,
+                            const gfx::Point& content_origin,
+                            const float& scale_factor) {
   DCHECK(context_.get());
   DCHECK(!page_is_open_);
 
@@ -82,7 +83,7 @@ CGContextRef PdfMetafile::StartPage(const gfx::Size& page_size,
   // Move the context to origin.
   CGContextTranslateCTM(context_, content_origin.x(), content_origin.y());
 
-  return context_.get();
+  return context_.get() != NULL;
 }
 
 bool PdfMetafile::FinishPage() {

diff --git a/printing/pdf_metafile_mac.h b/printing/pdf_metafile_mac.h
@@ -33,9 +33,9 @@ class PdfMetafile : public NativeMetafile {
   virtual bool Init();
   virtual bool InitFromData(const void* src_buffer, uint32 src_buffer_size);
 
-  virtual CGContextRef StartPage(const gfx::Size& page_size,
-                                 const gfx::Point& content_origin,
-                                 const float& scale_factor);
+  virtual bool StartPage(const gfx::Size& page_size,
+                         const gfx::Point& content_origin,
+                         const float& scale_factor);
   virtual bool FinishPage();
   virtual bool Close();
 

diff --git a/printing/pdf_ps_metafile_cairo.cc b/printing/pdf_ps_metafile_cairo.cc
@@ -145,23 +145,25 @@ bool PdfPsMetafile::SetRawData(const void* src_buffer,
   return true;
 }
 
-cairo_t* PdfPsMetafile::StartPage(const gfx::Size& page_size,
-                                  double margin_top_in_points,
-                                  double margin_left_in_points) {
+bool PdfPsMetafile::StartPage(const gfx::Size& page_size,
+                              const gfx::Point& content_origin,
+                              const float& scale_factor) {
   DCHECK(IsSurfaceValid(surface_));
   DCHECK(IsContextValid(context_));
   // Passing this check implies page_surface_ is NULL, and current_page_ is
   // empty.
   DCHECK_GT(page_size.width(), 0);
   DCHECK_GT(page_size.height(), 0);
+  // |scale_factor| is not supported yet.
+  DCHECK_EQ(scale_factor, 1);
 
   // Don't let WebKit draw over the margins.
   cairo_surface_set_device_offset(surface_,
-                                  margin_left_in_points,
-                                  margin_top_in_points);
+                                  content_origin.x(),
+                                  content_origin.y());
 
   cairo_pdf_surface_set_size(surface_, page_size.width(), page_size.height());
-  return context_;
+  return context_ != NULL;
 }
 
 bool PdfPsMetafile::FinishPage() {

diff --git a/printing/pdf_ps_metafile_cairo.h b/printing/pdf_ps_metafile_cairo.h
@@ -11,6 +11,12 @@
 #include "base/gtest_prod_util.h"
 #include "printing/native_metafile.h"
 
+namespace gfx {
+class Point;
+class Rect;
+class Size;
+}
+
 typedef struct _cairo_surface cairo_surface_t;
 
 namespace printing {
@@ -25,9 +31,9 @@ class PdfPsMetafile : public NativeMetafile {
   virtual bool Init();
   virtual bool InitFromData(const void* src_buffer, uint32 src_buffer_size);
 
-  virtual cairo_t* StartPage(const gfx::Size& page_size,
-                             double margin_top_in_points,
-                             double margin_left_in_points);
+  virtual bool StartPage(const gfx::Size& page_size,
+                         const gfx::Point& content_origin,
+                         const float& scale_factor);
   virtual bool FinishPage();
   virtual bool Close();
 

diff --git a/printing/pdf_ps_metafile_cairo_unittest.cc b/printing/pdf_ps_metafile_cairo_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/string_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/size.h"
+#include "ui/gfx/point.h"
 
 typedef struct _cairo cairo_t;
 
@@ -31,15 +32,13 @@ TEST_F(PdfPsTest, Pdf) {
   EXPECT_TRUE(pdf.Init());
 
   // Renders page 1.
-  cairo_t* context = pdf.StartPage(gfx::Size(72 + 2 + 4, 72 + 1 + 3), 1, 4);
-  EXPECT_TRUE(context != NULL);
-  EXPECT_EQ(printing::PdfPsMetafile::FromCairoContext(context), &pdf);
+  EXPECT_TRUE(pdf.StartPage(gfx::Size(72, 73), gfx::Point(4, 5), 1));
+  EXPECT_EQ(printing::PdfPsMetafile::FromCairoContext(pdf.context()), &pdf);
   // In theory, we should use Cairo to draw something on |context|.
   EXPECT_TRUE(pdf.FinishPage());
 
   // Renders page 2.
-  context = pdf.StartPage(gfx::Size(64 + 2 + 4, 64 + 1 + 3), 1, 4);
-  EXPECT_TRUE(context != NULL);
+  EXPECT_TRUE(pdf.StartPage(gfx::Size(72, 73), gfx::Point(4, 5), 1));
   // In theory, we should use Cairo to draw something on |context|.
   EXPECT_TRUE(pdf.FinishPage());
 
@@ -72,8 +71,7 @@ TEST_F(PdfPsTest, Pdf) {
   // Test overriding the metafile with raw data.
   printing::PdfPsMetafile pdf3;
   EXPECT_TRUE(pdf3.Init());
-  context = pdf3.StartPage(gfx::Size(72 + 2 + 4, 72 + 1 + 3), 1, 4);
-  EXPECT_TRUE(context != NULL);
+  EXPECT_TRUE(pdf3.StartPage(gfx::Size(72, 73), gfx::Point(4, 5), 1));
   std::string test_raw_data = "Dummy PDF";
   EXPECT_TRUE(pdf3.SetRawData(test_raw_data.c_str(), test_raw_data.size()));
   EXPECT_TRUE(pdf3.FinishPage());