Unifying NativeMetafile class interface (as much as possible) for Linux, Mac, Win

printing/emf_win.h

Index: printing/emf_win.h
diff --git a/printing/emf_win.h b/printing/emf_win.h
index ba139d873924df3822dd143b9d351829ffd0ba51..8afc21343763947959503141abb93378c150846e 100644
--- a/printing/emf_win.h
+++ b/printing/emf_win.h
@@ -10,7 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
-#include "printing/native_metafile_win.h"
+#include "printing/native_metafile.h"
 
 class FilePath;
 
@@ -20,7 +20,6 @@ class Rect;
 
 namespace printing {
 
-// Simple wrapper class that manage an EMF data stream and its virtual HDC.

[vandebo (ex-Chrome), 2011/03/14 20:24:25]

You can leave this comment in.

[vandebo (ex-Chrome), 2011/03/14 22:55:05]

You didn't address this.

[dpapad, 2011/03/15 16:11:06]

Done.

 class Emf : public NativeMetafile {
  public:
   class Record;
@@ -29,83 +28,50 @@ class Emf : public NativeMetafile {
 
   virtual ~Emf();
 
-  // Initializes the Emf with the data in |src_buffer|. Returns true on success.
+  //NativeMetafile methods.
+  virtual bool Init() { return true; }
   virtual bool Init(const void* src_buffer, uint32 src_buffer_size);
 
-  // Generates a virtual HDC that will record every GDI commands and compile it
-  // in a EMF data stream.
-  // hdc is used to setup the default DPI and color settings. hdc is optional.
-  // rect specifies the dimensions (in .01-millimeter units) of the EMF. rect is
-  // optional.
-  virtual bool CreateDc(HDC sibling, const RECT* rect);
+  virtual bool StartPage();
+  virtual bool FinishPage();
+  virtual bool Close();
 
-  // Similar to the above method but the metafile is backed by a file.
-  virtual bool CreateFileBackedDc(HDC sibling,
-                                  const RECT* rect,
-                                  const FilePath& path);
+  // Saves the EMF data to a file as-is. It is recommended to use the .emf file

[vandebo (ex-Chrome), 2011/03/14 20:24:25]

I think this comment goes with SaveTo ?

[dpapad, 2011/03/14 22:15:12]

Done. It turns out that moving functions around using Eclipse doesn't move the
comments too.

+  // extension but it is not enforced. This function synchronously writes to the
+  // file. For testing only.
+  virtual uint32 GetDataSize() const;
+  virtual bool GetData(void* buffer, uint32 size) const;
+
+  virtual bool SaveTo(const FilePath& file_path) const;
 
-    // Load an EMF file.
-  virtual bool CreateFromFile(const FilePath& metafile_path);
+  // Should be passed to Playback to keep the exact same size.
+  virtual gfx::Rect GetPageBounds(unsigned int page_number) const;
+
+  virtual unsigned int GetPageCount() const {
+    return 1;

[vandebo (ex-Chrome), 2011/03/14 20:24:25]

Actually (as I understand EMF better), we could have a multipage EMF, we just
don't right now.  Add a TODO to count the number of times StartPage is called. 
(You could fix this in the StartPage CL you're working on now.)

[dpapad, 2011/03/14 22:15:12]

Done.

+  }
 
-  // TODO(maruel): CreateFromFile(). If ever used. Maybe users would like to
-  // have the ability to save web pages to an EMF file? Afterward, it is easy to
-  // convert to PDF or PS.
+  virtual HDC context() const {
+    return hdc_;
+  }
 
-  // Closes the HDC created by CreateDc() and generates the compiled EMF
-  // data.
-  virtual bool CloseDc();
+  virtual bool CreateDc(HDC sibling, const RECT* rect);
+  virtual bool CreateFileBackedDc(HDC sibling,
+                                  const RECT* rect,
+                                  const FilePath& path);
+  virtual bool CreateFromFile(const FilePath& file_path);
 
-  // Closes the EMF data handle when it is not needed anymore.
   virtual void CloseEmf();
 
-  // "Plays" the EMF buffer in a HDC. It is the same effect as calling the
-  // original GDI function that were called when recording the EMF. |rect| is in
-  // "logical units" and is optional. If |rect| is NULL, the natural EMF bounds
-  // are used.
-  // Note: Windows has been known to have stack buffer overflow in its GDI
-  // functions, whether used directly or indirectly through precompiled EMF
-  // data. We have to accept the risk here. Since it is used only for printing,
-  // it requires user intervention.
   virtual bool Playback(HDC hdc, const RECT* rect) const;
-
-  // The slow version of Playback(). It enumerates all the records and play them
-  // back in the HDC. The trick is that it skip over the records known to have
-  // issue with some printers. See Emf::Record::SafePlayback implementation for
-  // details.
   virtual bool SafePlayback(HDC hdc) const;
 
-  // Retrieves the bounds of the painted area by this EMF buffer. This value
-  // should be passed to Playback to keep the exact same size.
-  virtual gfx::Rect GetBounds() const;
-
-  // Retrieves the EMF stream size.
-  virtual uint32 GetDataSize() const;
-
-  // Retrieves the EMF stream.
-  virtual bool GetData(void* buffer, uint32 size) const;
-
-  // Retrieves the EMF stream. It is an helper function.
   virtual bool GetData(std::vector<uint8>* buffer) const;
 
   virtual HENHMETAFILE emf() const {
     return emf_;
   }
 
-  virtual HDC hdc() const {
-    return hdc_;
-  }
-
-  // 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();
-  virtual bool EndPage();
-
-  // Saves the EMF data to a file as-is. It is recommended to use the .emf file
-  // extension but it is not enforced. This function synchronously writes to the
-  // file. For testing only.
-  virtual bool SaveTo(const std::wstring& filename) const;
-
  protected:
   Emf();
 
@@ -139,7 +105,7 @@ struct Emf::EnumerationContext {
 };
 
 // One EMF record. It keeps pointers to the EMF buffer held by Emf::emf_.
-// The entries become invalid once Emf::CloseEmf() is called.
+// The entries become invalid once Emf::Close() is called.

[vandebo (ex-Chrome), 2011/03/14 20:24:25]

CloseEmf

[dpapad, 2011/03/14 22:15:12]

Done.

 class Emf::Record {
  public:
   // Plays the record.
@@ -192,7 +158,7 @@ class Emf::Enumerator {
 
   // The collection of every EMF records in the currently loaded EMF buffer.
   // Initialized by Enumerate(). It keeps pointers to the EMF buffer held by
-  // Emf::emf_. The entries become invalid once Emf::CloseEmf() is called.
+  // Emf::emf_. The entries become invalid once Emf::Close() is called.

[vandebo (ex-Chrome), 2011/03/14 20:24:25]

CloseEmf

[dpapad, 2011/03/14 22:15:12]

Done.

   std::vector<Record> items_;
 
   EnumerationContext context_;