SkPDF: clean up skpdfdocument, add static functions

src/pdf/SkPDFDocument.h

Index: src/pdf/SkPDFDocument.h
diff --git a/src/pdf/SkPDFDocument.h b/src/pdf/SkPDFDocument.h
index e4c0521cbe0472bea31f0f783b1cbdbf0eee2568..fce9f9df834fe1f02a0ea793e87bdaab2900fa0b 100644
--- a/src/pdf/SkPDFDocument.h
+++ b/src/pdf/SkPDFDocument.h
@@ -29,34 +29,56 @@ template <typename T> class SkTSet;
 */
 class SkPDFDocument {

[mtklein, 2015/03/25 13:10:08]

It's starting to seem like SkPDFDocument doesn't need to exist.  It's becoming a
hanger for two static methods.  Is there more than one user of SkPDFDocument? 
If not, it seems like we should start migrating all this code to that single
user and delete this.

[hal.canary, 2015/03/25 13:19:45]

Options:
-  Keep this functionality together in a it's current .cpp
-  Rename the .cpp to SkPDFEmiter.cpp or something
-  move static functions to SkPDFDevice (already a very big file).

 public:
-    SkPDFDocument();
-    ~SkPDFDocument();
+    SkPDFDocument() {}
+    ~SkPDFDocument() { fPageDevices.unrefAll(); }

[mtklein, 2015/03/25 13:10:08]

This destructor seems like another good candidate to leave outlined.  It's not
really trivial work.  I guess it's okay if it's only called once, but then that
circles back to wondering why SkPDFDocument needs to exist.

[hal.canary, 2015/03/25 13:19:45]

It only gets called from one place (skdocument_pdf), and that class will take
direct ownership in a later cl.

 
     /** Output the PDF to the passed stream.  It is an error to call this (it
-     *  will return false and not modify stream) if no pages have been added
-     *  or there are pages missing (i.e. page 1 and 3 have been added, but not
-     *  page 2).
+     *  will return false and not modify stream) if pageDevices is empty.
+     *  No device pointer can be NULL.
+     *
+     *  @param pageDevices An array of pages, in order.  All pages
+     *                     should be created using the same SkPDFCanon.

[mtklein, 2015/03/25 13:10:08]

What happens if they're not?  Conflicting IDs?

I'm somewhat suspicious of classes which cannot enforce their own operational
invariants.  Seems like someone somewhere can enforce this structurally rather
than assert it, and this work should be done by them.

[hal.canary, 2015/03/25 13:19:45]

No de-duping across pages.  (a too-big pdf)
*You can not make this mistake from the public API.*

+     *                     TODO(halcanary): ASSERT this condition.
+     *  @param SkWStream   The writable output stream to send the PDF to.
+     */
+    static bool EmitPDF(const SkTDArray<SkPDFDevice*>& pageDevices, SkWStream*);
+
+    /** Output the PDF to the passed stream.  It is an error to call this (it
+     *  will return false and not modify stream) if no pages have been added.
      *
      *  @param stream    The writable output stream to send the PDF to.
      */
-    bool emitPDF(SkWStream* stream);
+    bool emitPDF(SkWStream* stream) const {
+        return SkPDFDocument::EmitPDF(fPageDevices, stream);
+    }
 
-    /** Append the passed pdf device to the document as a new page.  Returns
-     *  true if successful.  Will fail if the document has already been emitted.
+    /** Append the passed pdf device to the document as a new page.
      *
-     *  @param pdfDevice The page to add to this document.
+     *  @param pdfDevice The page to add to this document. All pages
+     *                   added to this document should be created
+     *                   using the same SkPDFCanon.
      */
-    bool appendPage(SkPDFDevice* pdfDevice) {
+    void appendPage(SkPDFDevice* pdfDevice) {
         fPageDevices.push(SkRef(pdfDevice));
-        return true;
     }
 
+    /** Get the count of unique font types used in the given pages.
+     */
+    static void GetCountOfFontTypes(
+            const SkTDArray<SkPDFDevice*>& pageDevices,
+            int counts[SkAdvancedTypefaceMetrics::kOther_Font + 1],
+            int* notSubsettableCount,
+            int* notEmbedddableCount);
+
     /** Get the count of unique font types used in the document.
      */
     void getCountOfFontTypes(
-        int counts[SkAdvancedTypefaceMetrics::kOther_Font + 1],
-        int* notSubsettableCount,
-        int* notEmbedddableCount) const;
+            int counts[SkAdvancedTypefaceMetrics::kOther_Font + 1],
+            int* notSubsettableCount,
+            int* notEmbedddableCount) const {
+        return SkPDFDocument::GetCountOfFontTypes(
+                fPageDevices, counts, notSubsettableCount, notEmbedddableCount);
+    }
 
 private:
     SkTDArray<SkPDFDevice*> fPageDevices;