See the tests on https://bugs.webkit.org/show_bug.cgi?id=108382. They pass for mac-wk1 (I tested locally) and fail for mac-wk2. Not sure if it's just a WebKitTestRunner issue or if it's also an issue in Safari. Either way, I'll skip the tests in TestExpectations for now as it looks to be a bug and not intentional. Looking at WebKit2/WebProcess/WebPage/WebPage.cpp, it definitely looks like the scale factor should be applying since we call m_printContext->begin(...).
*** Bug 144494 has been marked as a duplicate of this bug. ***
This is because WebKitTestRunner always takes the snapshots from the UI process (expect for IOS), so in the case of printing, the layout in the web view is not the expected one.
Created attachment 260360 [details] Patch
Created attachment 260361 [details] Try to fix CG build
Ping reviewers
Comment on attachment 260361 [details] Try to fix CG build View in context: https://bugs.webkit.org/attachment.cgi?id=260361&action=review Looks OK to me. > Source/WebCore/page/PrintContext.cpp:320 > +int PrintContext::numberOfPages(Frame* frame, const FloatSize& pageSizeInPixels) Should take a Frame&. The caller is the one that checks for null. > Source/WebCore/page/PrintContext.cpp:329 > +void PrintContext::spoolAllPagesWithBoundaries(Frame* frame, GraphicsContext& graphicsContext, const FloatSize& pageSizeInPixels) Should take a Frame&. The caller is the one that checks for null. > Source/WebCore/page/PrintContext.cpp:347 > +#if PLATFORM(COCOA) > graphicsContext.translate(0, totalHeight); > graphicsContext.scale(FloatSize(1, -1)); > +#endif Seems bizarre that this is here. The platform layer is responsible for abstracting differences like this. > Source/WebCore/page/PrintContext.cpp:357 > +#if PLATFORM(COCOA) > + int boundaryLineY = currentHeight; > +#else > + int boundaryLineY = currentHeight - 1; > +#endif Same comment about the platform layer.
(In reply to comment #6) > Comment on attachment 260361 [details] > Try to fix CG build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260361&action=review > > Looks OK to me. Thanks for the review. > > Source/WebCore/page/PrintContext.cpp:320 > > +int PrintContext::numberOfPages(Frame* frame, const FloatSize& pageSizeInPixels) > > Should take a Frame&. The caller is the one that checks for null. Will change that and the all the callers. > > Source/WebCore/page/PrintContext.cpp:329 > > +void PrintContext::spoolAllPagesWithBoundaries(Frame* frame, GraphicsContext& graphicsContext, const FloatSize& pageSizeInPixels) > > Should take a Frame&. The caller is the one that checks for null. Same here. > > Source/WebCore/page/PrintContext.cpp:347 > > +#if PLATFORM(COCOA) > > graphicsContext.translate(0, totalHeight); > > graphicsContext.scale(FloatSize(1, -1)); > > +#endif > > Seems bizarre that this is here. The platform layer is responsible for > abstracting differences like this. I was surprised as well, I guess this is specific to the printing code in mac. > > Source/WebCore/page/PrintContext.cpp:357 > > +#if PLATFORM(COCOA) > > + int boundaryLineY = currentHeight; > > +#else > > + int boundaryLineY = currentHeight - 1; > > +#endif > > Same comment about the platform layer.
Created attachment 261373 [details] Patch for landing
Committed r189908: <http://trac.webkit.org/changeset/189908>
This broke "run-webkit-tests --pixel" behavior, at least on Mac. How it is supposed to work is that --pixel has no effect on tests that call testRunner.dumpAsText() or testRunner.dumpAsText(false). But now, WebKitTestRunner dumps pixels for these, so all text-only tests fail when run in this mode. This breaks an important testing scenario, so I'll roll out for now.
Re-opened since this is blocked by bug 149368
If the fix is straightforward, please feel free to re-land without a new review round.
(In reply to comment #10) > This broke "run-webkit-tests --pixel" behavior, at least on Mac. How it is > supposed to work is that --pixel has no effect on tests that call > testRunner.dumpAsText() or testRunner.dumpAsText(false). But now, > WebKitTestRunner dumps pixels for these, so all text-only tests fail when > run in this mode. > > This breaks an important testing scenario, so I'll roll out for now. Oh, I'm sorry about this, I got confused with all the shouldDumpPixels in TestInvocation, TestRunner, InjectedBundle and InjectedBundlePage. Now that I understand it, I'm going to split the patch, first to avoid creating snapshots in the web process that will be ignored, and then the one to fix the printing snapshots.
Created attachment 262050 [details] Patch for landing
Committed r190306: <http://trac.webkit.org/changeset/190306>