[Webkit-unassigned] [Bug 61256] REGRESSION (r77257): Print HTML with only 1 line, then 2 pages are printed

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61256


Kentaro Hara <haraken at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #96527|1                           |0
        is obsolete|                            |
  Attachment #96527|                            |commit-queue?
               Flag|                            |




--- Comment #41 from Kentaro Hara <haraken at google.com>  2011-06-09 22:27:12 PST ---
(From update of attachment 96527)
View in context: https://bugs.webkit.org/attachment.cgi?id=96527&action=review

>> LayoutTests/ChangeLog:14
>> +        * printing/page-count-with-one-word.html: Added.
> 
> How much time does the test take now? I think that we should aim towards under 1 second in debug builds.

In my environment, 1 second on Mac Safari, Mac Chromium and Linux Chromium, and 2 seconds on Windows Chromium. 

I choose [530px, 560px) for width and [730px, 760px) for height, because 559px x 735px is used in Mac Safari and 539px x 755px is used in Linux Chromium, although these values highly depends on OSes, browsers and printer drivers. If you think that the number of tests is still too many, I will more investigate the page sizes used in other settings in order to reduce the number of tests without degrading the quality of the test.

>> LayoutTests/printing/page-count-with-one-word-expected.txt:5
>> +width=530, height=732: numberOfPages=1
> 
> Can you omit all these lines from the output, and only log failures if they occur (and a PASS/FAIL overall result, of course)?

Done.

>> Source/WebCore/page/Frame.cpp:571
>> +void Frame::resizePageRectsKeepingRatio(float originalWidth, float originalHeight, float expectedWidth, float expectedHeight, float& resultWidth, float& resultHeight)
> 
> Would FloatSize work for the arguments?

Yes, I changed them to FloatSizes.

>> Source/WebCore/page/Frame.cpp:574
>> +        return;
> 
> How will the caller know about the failure? In your code, callers will be just using uninitialized memory from stack in this situation.
> 
> You need to either return an error (and handle it in callers), assume that a renderer is always present, or approximate the result when there is none.

I fixed it so that this method returns [0, 0] as a result when contentRenderer() fails.

>> Source/WebCore/page/PrintContext.cpp:166
>> +        return;
> 
> Can this actually happen?
> 
> It's probably insufficient to just return from end() - the caller will proceed with printing, calling end() etc, which will do no good.

I think it cannot happen. I just removed it.

>> Source/WebKit/mac/WebView/WebHTMLViewPrivate.h:135
>> +- (BOOL)_beginPrintModeWithMinimumPageWidth:(WebCGFloat)minimumPageWidth height:(WebCGFloat)minimumPageHeight maximumShrinkRatio:(WebCGFloat)maximumShrinkRatio;
> 
> This method's signature cannot be changed. Can you find a way to fix the problem without changing it?

I see. I reverted the signature of this method.

>> Source/WebKit/win/WebFrame.cpp:2034
>> +        ASSERT(coreFrame);
> 
> I know that you copied this from another method, but we don't usually assert what is going to be checked by an actual call right below the assertion anyway. It does not really matter if you crash on an assertion here, or if you crash inside the resizePageRectsKeepingRatio() call.

I removed this assertion.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.