Fork of https://codereview.chromium.org/469833003/

Cannot print PDFs

In r180112, I removed some seemly dead code, but that code actually existed to
make some functions on PrintContext virtual. Instead of hacking in virtuals via
a subclass in the web layer, we can just make the functions virtual in the base
class.

I've also added OVERRIDE annotations to catch this sort of error at compile
time. These annotations required me to fix the return type of one of these
functions to match. Finally, I've added a test, which appears to be the first
unit test to exercise this codepath.

BUG=403467

Review URL: https://codereview.chromium.org/476463002

git-svn-id: svn://svn.chromium.org/blink/trunk@180220 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/page/PrintContext.cpp

@@ -54,7 +54,7 @@ PrintContext::~PrintContext()
         end();
 }
 
-void PrintContext::computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight, bool allowHorizontalTiling)
+void PrintContext::computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight)
 {
     m_pageRects.clear();
     outPageHeight = 0;
@@ -81,7 +81,7 @@ void PrintContext::computePageRects(const FloatRect& printRect, float headerHeig
         return;
     }
 
-    computePageRectsWithPageSizeInternal(FloatSize(pageWidth / userScaleFactor, pageHeight / userScaleFactor), allowHorizontalTiling);
+    computePageRectsWithPageSizeInternal(FloatSize(pageWidth / userScaleFactor, pageHeight / userScaleFactor), false);
 }
 
 void PrintContext::computePageRectsWithPageSize(const FloatSize& pageSizeInPixels, bool allowHorizontalTiling)

third_party/WebKit/Source/core/page/PrintContext.h

@@ -40,7 +40,7 @@ class Node;
 class PrintContext : public NoBaseWillBeGarbageCollectedFinalized<PrintContext> {
 public:
     explicit PrintContext(LocalFrame*);
-    ~PrintContext();
+    virtual ~PrintContext();
 
     LocalFrame* frame() const { return m_frame; }
 
@@ -48,10 +48,11 @@ public:
     // FIXME: This means that CSS page breaks won't be on page boundary if the size is different than what was passed to begin(). That's probably not always desirable.
     // FIXME: Header and footer height should be applied before layout, not after.
     // FIXME: The printRect argument is only used to determine page aspect ratio, it would be better to pass a FloatSize with page dimensions instead.
-    void computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight, bool allowHorizontalTiling = false);
+    virtual void computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight);
 
     // Deprecated. Page size computation is already in this class, clients shouldn't be copying it.
-    void computePageRectsWithPageSize(const FloatSize& pageSizeInPixels, bool allowHorizontalTiling);
+    // FIXME: Everyone passes |false| for the second paramer. We should remove the second parameter.
+    virtual void computePageRectsWithPageSize(const FloatSize& pageSizeInPixels, bool allowHorizontalTiling);
 
     // These are only valid after page rects are computed.
     size_t pageCount() const { return m_pageRects.size(); }
@@ -60,10 +61,10 @@ public:
 
     // Enter print mode, updating layout for new page size.
     // This function can be called multiple times to apply new print options without going back to screen mode.
-    void begin(float width, float height = 0);
+    virtual void begin(float width, float height = 0);
 
     // Return to screen mode.
-    void end();
+    virtual void end();
 
     // Used by layout tests.
     static int pageNumberForElement(Element*, const FloatSize& pageSizeInPixels); // Returns -1 if page isn't found.

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

@@ -396,43 +396,43 @@ private:
 class ChromePluginPrintContext FINAL : public ChromePrintContext {
 public:
     ChromePluginPrintContext(LocalFrame* frame, WebPluginContainerImpl* plugin, const WebPrintParams& printParams)
-        : ChromePrintContext(frame), m_plugin(plugin), m_pageCount(0), m_printParams(printParams)
+        : ChromePrintContext(frame), m_plugin(plugin), m_printParams(printParams)
     {
     }
 
     virtual ~ChromePluginPrintContext() { }
 
-    virtual void begin(float width, float height)
+    virtual void begin(float width, float height) OVERRIDE
     {
     }
 
-    virtual void end()
+    virtual void end() OVERRIDE
     {
         m_plugin->printEnd();
     }
 
-    virtual float getPageShrink(int pageNumber) const
+    virtual float getPageShrink(int pageNumber) const OVERRIDE
     {
         // We don't shrink the page (maybe we should ask the widget ??)
         return 1.0;
     }
 
-    virtual void computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight)
+    virtual void computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight) OVERRIDE
     {
         m_printParams.printContentArea = IntRect(printRect);
-        m_pageCount = m_plugin->printBegin(m_printParams);
+        m_pageRects.fill(IntRect(printRect), m_plugin->printBegin(m_printParams));
     }
 
-    virtual int pageCount() const
+    virtual void computePageRectsWithPageSize(const FloatSize& pageSizeInPixels, bool allowHorizontalTiling) OVERRIDE
     {
-        return m_pageCount;
+        ASSERT_NOT_REACHED();
     }
 
 protected:
     // Spools the printed page, a subrect of frame(). Skip the scale step.
     // NativeTheme doesn't play well with scaling. Scaling is done browser side
     // instead. Returns the scale to be applied.
-    virtual float spoolPage(GraphicsContext& context, int pageNumber)
+    virtual float spoolPage(GraphicsContext& context, int pageNumber) OVERRIDE
     {
         m_plugin->printPage(pageNumber, &context);
         return 1.0;
@@ -441,9 +441,7 @@ protected:
 private:
     // Set when printing.
     WebPluginContainerImpl* m_plugin;
-    int m_pageCount;
     WebPrintParams m_printParams;
-
 };
 
 static WebDataSource* DataSourceForDocLoader(DocumentLoader* loader)
@@ -1281,7 +1279,7 @@ int WebLocalFrameImpl::printBegin(const WebPrintParams& printParams, const WebNo
     // browser. pageHeight is actually an output parameter.
     m_printContext->computePageRects(rect, 0, 0, 1.0, pageHeight);
 
-    return m_printContext->pageCount();
+    return static_cast<int>(m_printContext->pageCount());
 }
 
 float WebLocalFrameImpl::getPrintPageShrink(int page)

third_party/WebKit/Source/web/tests/WebFrameTest.cpp

@@ -82,6 +82,7 @@
 #include "public/web/WebFormElement.h"
 #include "public/web/WebFrameClient.h"
 #include "public/web/WebHistoryItem.h"
+#include "public/web/WebPrintParams.h"
 #include "public/web/WebRange.h"
 #include "public/web/WebRemoteFrame.h"
 #include "public/web/WebScriptSource.h"
@@ -5801,6 +5802,22 @@ TEST_F(WebFrameTest, NodeImageTestFloatLeft)
     nodeImageTestValidation(blink::IntSize(40, 40), dragImage.get());
 }
 
+TEST_F(WebFrameTest, PrintingBasic)
+{
+    FrameTestHelpers::WebViewHelper webViewHelper;
+    webViewHelper.initializeAndLoad("data:text/html,Hello, world.");
+
+    WebFrame* frame = webViewHelper.webView()->mainFrame();
+
+    WebPrintParams printParams;
+    printParams.printContentArea.width = 500;
+    printParams.printContentArea.height = 500;
+
+    int pageCount = frame->printBegin(printParams);
+    EXPECT_EQ(1, pageCount);
+    frame->printEnd();
+}
+
 class ThemeColorTestWebFrameClient : public FrameTestHelpers::TestWebFrameClient {
 public:
     ThemeColorTestWebFrameClient()