Bug 105519 - Reducing print preview scale significantly causes a crash
Summary: Reducing print preview scale significantly causes a crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Printing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-12-20 02:31 PST by Tim Horton
Modified: 2012-12-21 01:56 PST (History)
1 user (show)

See Also:


Attachments
patch (11.02 KB, patch)
2012-12-20 02:46 PST, Tim Horton
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2012-12-20 02:31:06 PST
The print preview image snapshot is being generated with a size that takes the print preview scale into account. It doesn't need to do this - the bitmap just needs to be the size that we're going to draw at!

<rdar://problem/12807090>
Comment 1 Tim Horton 2012-12-20 02:46:27 PST
Created attachment 180308 [details]
patch
Comment 2 Alexey Proskuryakov 2012-12-20 09:36:38 PST
Comment on attachment 180308 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180308&action=review

> Source/WebKit2/ChangeLog:12
> +        The bitmap image used for print previews was being created respecting the print preview scale.
> +        This doesn't make any sense, as the scale does not affect the size of the image required to
> +        represent the previewed page. Instead, we should not scale the size, creating the buffer at a
> +        size that is constant regardless of scale, and do the scaling when drawing *into* the buffer instead.

With four lines of explanation, I'm still not quite sure if this was an out of memory crash :)

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3283
> +        float printingScale = (float)desiredImageSize.width() / rect.width();

We generally prefer static_cast.

> Source/WebKit2/WebProcess/WebPage/WebPage.messages.in:220
> +    DrawRectToImage(uint64_t frameID, WebKit::PrintInfo printInfo, WebCore::IntRect rect, WebCore::IntSize desiredImageSize, uint64_t callbackID)

"Desired" makes it sound like the desire may not be granted. Is that accurate?
Comment 3 Tim Horton 2012-12-20 10:13:07 PST
(In reply to comment #2)
> (From update of attachment 180308 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180308&action=review
> 
> > Source/WebKit2/ChangeLog:12
> > +        The bitmap image used for print previews was being created respecting the print preview scale.
> > +        This doesn't make any sense, as the scale does not affect the size of the image required to
> > +        represent the previewed page. Instead, we should not scale the size, creating the buffer at a
> > +        size that is constant regardless of scale, and do the scaling when drawing *into* the buffer instead.
> 
> With four lines of explanation, I'm still not quite sure if this was an out of memory crash :)

It was, but the crash was secondary to the fact that we were respecting the scale at all when we didn't need to :D

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3283
> > +        float printingScale = (float)desiredImageSize.width() / rect.width();
> 
> We generally prefer static_cast.

Of course!

> > Source/WebKit2/WebProcess/WebPage/WebPage.messages.in:220
> > +    DrawRectToImage(uint64_t frameID, WebKit::PrintInfo printInfo, WebCore::IntRect rect, WebCore::IntSize desiredImageSize, uint64_t callbackID)
> 
> "Desired" makes it sound like the desire may not be granted. Is that accurate?

No, it will always be the desired size. Perhaps requested? or just imageSize?
Comment 4 Tim Horton 2012-12-21 01:56:02 PST
http://trac.webkit.org/changeset/138356